From d8a1ffae81ff934c25ba05652f1106b2f4226479 Mon Sep 17 00:00:00 2001 From: Nico <32375741+nulmete@users.noreply.github.com> Date: Thu, 7 May 2026 16:10:15 +0200 Subject: [PATCH] Clear stale broken label rows on profile batch upsert (#44847) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **Related issue:** Resolves #42637 # Checklist for submitter - [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/guides/committing-changes.md#changes-files) for more information. ## Testing - [x] Added/updated automated tests - [x] QA'd all new/changed functionality manually ### Reproduction steps: - Created Label X and Label Y as manual labels in the UI. - Applied gitops referencing the labels. The specified profile referenced Label X: ```yaml macos_settings: custom_settings: - path: ../repro-42637-profile.mobileconfig labels_exclude_any: - "Repro Label X 42637" ``` - Manually ran a SQL query to update `label_id` to NULL. Screenshot 2026-05-06 at 6 19 51 PM - Re-ran gitops referencing Label Y in the profile config. ```yaml macos_settings: custom_settings: - path: ../repro-42637-profile.mobileconfig labels_include_any: - "Repro Label Y 42637" ``` - Old row was preserved AND a new one was created (association to Label Y): Screenshot 2026-05-06 at 6 22 07 PM ### Testing steps - Re-built fleetctl with the fix applied and re-ran gitops, still referencing Label Y for the profile. - Confirmed the orphan row was deleted. Screenshot 2026-05-06 at 6 24 43 PM ## Summary by CodeRabbit ## Bug Fixes * Fixed an issue where MDM configuration profiles would remain enforced on hosts after their associated labels were deleted during fleetctl gitops apply operations. Label associations are now properly cleared when profiles are reapplied with updated targeting. --- ...2637-clear-stale-broken-label-associations | 1 + server/datastore/mysql/apple_mdm_test.go | 66 ++++++++++++++++ server/datastore/mysql/mdm.go | 9 ++- server/datastore/mysql/mdm_test.go | 75 +++++++++++++++++++ 4 files changed, 148 insertions(+), 3 deletions(-) create mode 100644 changes/42637-clear-stale-broken-label-associations diff --git a/changes/42637-clear-stale-broken-label-associations b/changes/42637-clear-stale-broken-label-associations new file mode 100644 index 0000000000..d977c03644 --- /dev/null +++ b/changes/42637-clear-stale-broken-label-associations @@ -0,0 +1 @@ +* Fixed `fleetctl gitops apply` not clearing stale broken `mdm_configuration_profile_labels` rows after a referenced label was deleted, which caused profiles to remain enforced on hosts regardless of updated label targeting. diff --git a/server/datastore/mysql/apple_mdm_test.go b/server/datastore/mysql/apple_mdm_test.go index 760c5afde6..e43927463c 100644 --- a/server/datastore/mysql/apple_mdm_test.go +++ b/server/datastore/mysql/apple_mdm_test.go @@ -55,6 +55,7 @@ func TestMDMApple(t *testing.T) { {"TestHostDetailsMDMProfiles", testHostDetailsMDMProfiles}, {"TestHostDetailsMDMProfilesIOSIPadOS", testHostDetailsMDMProfilesIOSIPadOS}, {"TestBatchSetMDMAppleProfiles", testBatchSetMDMAppleProfiles}, + {"TestBatchSetMDMAppleProfilesClearsStaleBrokenLabels", testBatchSetMDMAppleProfilesClearsStaleBrokenLabels}, {"TestMDMAppleProfileManagement", testMDMAppleProfileManagement}, {"TestMDMAppleProfileManagementBatch2", testMDMAppleProfileManagementBatch2}, {"TestMDMAppleProfileManagementBatch3", testMDMAppleProfileManagementBatch3}, @@ -1489,6 +1490,71 @@ func testBatchSetMDMAppleProfiles(t *testing.T, ds *Datastore) { applyAndExpect(nil, ptr.Uint(1), expectFleetProfiles) } +// Regression test for https://github.com/fleetdm/fleet/issues/42637. +func testBatchSetMDMAppleProfilesClearsStaleBrokenLabels(t *testing.T, ds *Datastore) { + ctx := t.Context() + + labelX, err := ds.NewLabel(ctx, &fleet.Label{Name: "exclude-x-42637", Query: "select 1 from osquery_info;"}) + require.NoError(t, err) + labelY, err := ds.NewLabel(ctx, &fleet.Label{Name: "include-any-y-42637", Query: "select 1 from osquery_info;"}) + require.NoError(t, err) + + // First gitops apply: profile with labels_exclude_any: [labelX]. + prof := configProfileForTest(t, "Repro42637", "com.fleetdm.repro.42637", "u1", labelX) + require.NoError(t, ds.BatchSetMDMAppleProfiles(ctx, nil, []*fleet.MDMAppleConfigProfile{prof})) + + var profileUUID string + ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error { + return sqlx.GetContext(ctx, q, &profileUUID, + `SELECT profile_uuid FROM mdm_apple_configuration_profiles WHERE identifier = ?`, + prof.Identifier) + }) + require.NotEmpty(t, profileUUID) + + type row struct { + LabelID *uint `db:"label_id"` + LabelName string `db:"label_name"` + Exclude bool `db:"exclude"` + } + loadRows := func() []row { + var rows []row + ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error { + return sqlx.SelectContext(ctx, q, &rows, + `SELECT label_id, label_name, exclude FROM mdm_configuration_profile_labels WHERE apple_profile_uuid = ? ORDER BY label_name`, + profileUUID) + }) + return rows + } + + rows := loadRows() + require.Len(t, rows, 1) + require.NotNil(t, rows[0].LabelID) + require.Equal(t, labelX.ID, *rows[0].LabelID) + require.Equal(t, labelX.Name, rows[0].LabelName) + require.True(t, rows[0].Exclude) + + // Simulate labelX being deleted. + ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error { + _, err := q.ExecContext(ctx, + `UPDATE mdm_configuration_profile_labels SET label_id = NULL WHERE apple_profile_uuid = ? AND label_name = ?`, + profileUUID, labelX.Name) + return err + }) + rows = loadRows() + require.Len(t, rows, 1) + require.Nil(t, rows[0].LabelID, "row should be in broken (NULL label_id) state") + + prof = configProfileForTest(t, "Repro42637", "com.fleetdm.repro.42637", "u1", labelY) + require.NoError(t, ds.BatchSetMDMAppleProfiles(ctx, nil, []*fleet.MDMAppleConfigProfile{prof})) + + rows = loadRows() + require.Len(t, rows, 1, "stale broken row was not cleared") + require.NotNil(t, rows[0].LabelID) + require.Equal(t, labelY.ID, *rows[0].LabelID) + require.Equal(t, labelY.Name, rows[0].LabelName) + require.False(t, rows[0].Exclude) +} + func configProfileBytesForTest(name, identifier, uuid string) []byte { return []byte(fmt.Sprintf(` diff --git a/server/datastore/mysql/mdm.go b/server/datastore/mysql/mdm.go index f39341c041..499658e78c 100644 --- a/server/datastore/mysql/mdm.go +++ b/server/datastore/mysql/mdm.go @@ -1710,12 +1710,15 @@ func batchSetProfileLabelAssociationsDB( return false, fmt.Errorf("unsupported platform %s", platform) } - // delete any profile+label tuple that is NOT in the list of provided tuples + // Delete any profile+label tuple that is NOT in the list of provided tuples // but are associated with the provided profiles (so we don't delete - // unrelated profile+label tuples) + // unrelated profile+label tuples). + // Also clear "broken" rows where label_id IS NULL. those wouldn't be matched by + // `(profile_uuid, label_id) NOT IN (...)` because three-valued logic makes the comparison NULL, + // leaving the row in place and blocking host removal in generateEntitiesToRemoveQuery. deleteStmt := ` DELETE FROM mdm_configuration_profile_labels - WHERE (%s_profile_uuid, label_id) NOT IN (%s) AND + WHERE ((%s_profile_uuid, label_id) NOT IN (%s) OR label_id IS NULL) AND %s_profile_uuid IN (?) ` diff --git a/server/datastore/mysql/mdm_test.go b/server/datastore/mysql/mdm_test.go index 7e0cdcad01..3d2ca3edb3 100644 --- a/server/datastore/mysql/mdm_test.go +++ b/server/datastore/mysql/mdm_test.go @@ -7686,6 +7686,81 @@ func testBatchSetProfileLabelAssociations(t *testing.T, ds *Datastore) { require.NoError(t, err) expectLabels(t, uuid, platform, nil) }) + + t.Run("broken label association is cleared on upsert "+platform, func(t *testing.T) { + // Regression test for https://github.com/fleetdm/fleet/issues/42637. + startingLabel := &fleet.Label{ + Name: "broken-label-" + platform, + Query: "select 1 from osquery_info;", + } + startingLabel, err := ds.NewLabel(ctx, startingLabel) + require.NoError(t, err) + + profileLabels := []fleet.ConfigurationProfileLabel{ + {ProfileUUID: uuid, LabelName: startingLabel.Name, LabelID: startingLabel.ID, Exclude: true}, + } + err = ds.withTx(ctx, func(tx sqlx.ExtContext) error { + _, err := batchSetProfileLabelAssociationsDB(ctx, tx, profileLabels, nil, platform) + return err + }) + require.NoError(t, err) + expectLabels(t, uuid, platform, profileLabels) + + // Simulate the label being deleted: NULL the label_id directly. This is + // what FK ON DELETE SET NULL does when the underlying label row is gone. + p := platform + if p == "darwin" { + p = "apple" + } + ExecAdhocSQL(t, ds, func(tx sqlx.ExtContext) error { + _, err := tx.ExecContext(ctx, + fmt.Sprintf(`UPDATE mdm_configuration_profile_labels SET label_id = NULL WHERE %s_profile_uuid = ? AND label_name = ?`, p), + uuid, startingLabel.Name) + return err + }) + + // Sanity: broken row exists. + var brokenCount int + ExecAdhocSQL(t, ds, func(tx sqlx.ExtContext) error { + return sqlx.GetContext(ctx, tx, &brokenCount, + fmt.Sprintf(`SELECT COUNT(*) FROM mdm_configuration_profile_labels WHERE %s_profile_uuid = ? AND label_id IS NULL`, p), + uuid) + }) + require.Equal(t, 1, brokenCount, "expected broken row to exist before re-upsert") + + // Re-apply with a different label, mimicking gitops switching from + // labels_exclude_any: [startingLabel] to labels_include_any: [switchTarget]. + switchTarget := &fleet.Label{ + Name: "switch-target-" + platform, + Query: "select 1 from osquery_info;", + } + switchTarget, err = ds.NewLabel(ctx, switchTarget) + require.NoError(t, err) + + profileLabels = []fleet.ConfigurationProfileLabel{ + {ProfileUUID: uuid, LabelName: switchTarget.Name, LabelID: switchTarget.ID, Exclude: false}, + } + err = ds.withTx(ctx, func(tx sqlx.ExtContext) error { + _, err := batchSetProfileLabelAssociationsDB(ctx, tx, profileLabels, nil, platform) + return err + }) + require.NoError(t, err) + + // The broken row must be gone + ExecAdhocSQL(t, ds, func(tx sqlx.ExtContext) error { + return sqlx.GetContext(ctx, tx, &brokenCount, + fmt.Sprintf(`SELECT COUNT(*) FROM mdm_configuration_profile_labels WHERE %s_profile_uuid = ? AND label_id IS NULL`, p), + uuid) + }) + require.Equal(t, 0, brokenCount, "broken (NULL label_id) row should have been cleared") + + // Only switchTarget should remain. + expectLabels(t, uuid, platform, profileLabels) + + // Other profiles must remain untouched. + expectLabels(t, otherWinProfile.ProfileUUID, "windows", wantOtherWin) + expectLabels(t, otherMacProfile.ProfileUUID, "darwin", wantOtherMac) + }) } t.Run("unsupported platform", func(t *testing.T) {