From 050e771635b056b8d8ab616b7b3f9d0887ce8acc Mon Sep 17 00:00:00 2001 From: Jahziel Villasana-Espinoza Date: Fri, 18 Oct 2024 15:16:58 -0400 Subject: [PATCH] fix: remove declarations that haven't been sent yet (#22993) > Related issue: #22976 # Checklist for submitter If some of the following don't apply, delete the relevant line. - [x] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/Committing-Changes.md#changes-files) for more information. - [x] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements) - [x] Added/updated tests - [x] Manual QA for all new/changed functionality --- changes/22976-ddm-delete | 2 ++ server/datastore/mysql/apple_mdm.go | 32 ++++++++++++++++++---- server/datastore/mysql/apple_mdm_test.go | 3 +- server/service/integration_mdm_ddm_test.go | 13 +++++++++ 4 files changed, 43 insertions(+), 7 deletions(-) create mode 100644 changes/22976-ddm-delete diff --git a/changes/22976-ddm-delete b/changes/22976-ddm-delete new file mode 100644 index 0000000000..7c355b79a7 --- /dev/null +++ b/changes/22976-ddm-delete @@ -0,0 +1,2 @@ +- Fixes a bug where DDM declarations would remaing "pending" forever if they were deleted + from Fleet before being sent to hosts. \ No newline at end of file diff --git a/server/datastore/mysql/apple_mdm.go b/server/datastore/mysql/apple_mdm.go index 38fed9ff92..8cf0d67658 100644 --- a/server/datastore/mysql/apple_mdm.go +++ b/server/datastore/mysql/apple_mdm.go @@ -299,7 +299,18 @@ func (ds *Datastore) DeleteMDMAppleConfigProfileByDeprecatedID(ctx context.Conte func (ds *Datastore) DeleteMDMAppleConfigProfile(ctx context.Context, profileUUID string) error { // TODO(roberto): this seems confusing to me, we should have a separate datastore method. if strings.HasPrefix(profileUUID, fleet.MDMAppleDeclarationUUIDPrefix) { - return ds.deleteMDMAppleDeclaration(ctx, profileUUID) + return ds.withRetryTxx(ctx, func(tx sqlx.ExtContext) error { + if err := deleteMDMAppleDeclaration(ctx, tx, profileUUID); err != nil { + return err + } + + if err := deleteUnsentAppleHostMDMDeclaration(ctx, tx, profileUUID); err != nil { + return err + } + + return nil + }) + // return ds.deleteMDMAppleDeclaration(ctx, profileUUID) } return ds.withRetryTxx(ctx, func(tx sqlx.ExtContext) error { if err := deleteMDMAppleConfigProfileByIDOrUUID(ctx, tx, 0, profileUUID); err != nil { @@ -349,6 +360,15 @@ func deleteUnsentAppleHostMDMProfile(ctx context.Context, tx sqlx.ExtContext, uu return nil } +func deleteUnsentAppleHostMDMDeclaration(ctx context.Context, tx sqlx.ExtContext, uuid string) error { + const stmt = `DELETE FROM host_mdm_apple_declarations WHERE declaration_uuid = ? AND status IS NULL AND operation_type = ?` + if _, err := tx.ExecContext(ctx, stmt, uuid, fleet.MDMOperationTypeInstall); err != nil { + return ctxerr.Wrap(ctx, err, "deleting host declaration that has not been sent to host") + } + + return nil +} + func (ds *Datastore) DeleteMDMAppleDeclarationByName(ctx context.Context, teamID *uint, name string) error { const stmt = `DELETE FROM mdm_apple_declarations WHERE team_id = ? AND name = ?` @@ -363,10 +383,10 @@ func (ds *Datastore) DeleteMDMAppleDeclarationByName(ctx context.Context, teamID return nil } -func (ds *Datastore) deleteMDMAppleDeclaration(ctx context.Context, uuid string) error { +func deleteMDMAppleDeclaration(ctx context.Context, tx sqlx.ExtContext, uuid string) error { stmt := `DELETE FROM mdm_apple_declarations WHERE declaration_uuid = ?` - res, err := ds.writer(ctx).ExecContext(ctx, stmt, uuid) + res, err := tx.ExecContext(ctx, stmt, uuid) if err != nil { return ctxerr.Wrap(ctx, err) } @@ -456,7 +476,8 @@ WHERE } func (ds *Datastore) GetHostMDMCertificateProfile(ctx context.Context, hostUUID string, - profileUUID string) (*fleet.HostMDMCertificateProfile, error) { + profileUUID string, +) (*fleet.HostMDMCertificateProfile, error) { stmt := ` SELECT hmap.host_uuid, @@ -4781,7 +4802,8 @@ func (ds *Datastore) InsertMDMConfigAssets(ctx context.Context, assets []fleet.M } func (ds *Datastore) GetAllMDMConfigAssetsByName(ctx context.Context, assetNames []fleet.MDMAssetName, - queryerContext sqlx.QueryerContext) (map[fleet.MDMAssetName]fleet.MDMConfigAsset, error) { + queryerContext sqlx.QueryerContext, +) (map[fleet.MDMAssetName]fleet.MDMConfigAsset, error) { if len(assetNames) == 0 { return nil, nil } diff --git a/server/datastore/mysql/apple_mdm_test.go b/server/datastore/mysql/apple_mdm_test.go index 7c4f58661e..8ac54bb87c 100644 --- a/server/datastore/mysql/apple_mdm_test.go +++ b/server/datastore/mysql/apple_mdm_test.go @@ -4927,7 +4927,7 @@ func testMDMAppleDDMDeclarationsToken(t *testing.T, ds *Datastore) { updates, err = ds.BulkSetPendingMDMHostProfiles(ctx, nil, nil, []string{decl2.DeclarationUUID}, nil) require.NoError(t, err) assert.False(t, updates.AppleConfigProfile) - assert.True(t, updates.AppleDeclaration) + assert.False(t, updates.AppleDeclaration) // This is false because we delete references in `host_mdm_apple_declarations` for declarations that aren't sent to the host assert.False(t, updates.WindowsConfigProfile) toks, err = ds.MDMAppleDDMDeclarationsToken(ctx, host1.UUID) @@ -7217,5 +7217,4 @@ func testMDMManagedCertificates(t *testing.T, ds *Datastore) { badProfileUUID) }) require.ErrorIs(t, err, sql.ErrNoRows) - } diff --git a/server/service/integration_mdm_ddm_test.go b/server/service/integration_mdm_ddm_test.go index 34a4956a4d..25035917b5 100644 --- a/server/service/integration_mdm_ddm_test.go +++ b/server/service/integration_mdm_ddm_test.go @@ -565,6 +565,19 @@ func (s *integrationMDMTestSuite) TestAppleDDMReconciliation() { // create a host and then enroll in MDM. mdmHost, device := createHostThenEnrollMDM(s.ds, s.server.URL, t) + // Create and then immediately delete a declaration + delUUID := addDeclaration("TestImmediateDelete", 0, nil) + var hostResp getHostResponse + s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/hosts/%d", mdmHost.ID), nil, http.StatusOK, &hostResp) + require.NotNil(t, hostResp.Host.MDM.Profiles) + require.Len(t, *hostResp.Host.MDM.Profiles, 1) + require.Equal(t, (*hostResp.Host.MDM.Profiles)[0].Name, "TestImmediateDelete") + + deleteDeclaration(delUUID) + hostResp = getHostResponse{} + s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/hosts/%d", mdmHost.ID), nil, http.StatusOK, &hostResp) + require.Nil(t, hostResp.Host.MDM.Profiles) + // trigger the reconciler, no error err = ReconcileAppleDeclarations(ctx, s.ds, s.mdmCommander, s.logger) require.NoError(t, err)