Discharge7443/fleet
1
0
Fork 0
Code Issues Pull Requests Actions 19 Packages Projects Releases Wiki Activity

Clear stale broken label rows on profile batch upsert (#44847)

Browse Source 
<!-- Add the related story/sub-task/bug number, like Resolves #123, or
remove if NA -->
**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.

<img width="712" height="46" alt="Screenshot 2026-05-06 at 6 19 51 PM"
src="https://github.com/user-attachments/assets/32f386c7-adf3-48e8-adee-03102831e556"
/>


- 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):

<img width="709" height="68" alt="Screenshot 2026-05-06 at 6 22 07 PM"
src="https://github.com/user-attachments/assets/fe2c4644-eb95-45a0-a582-994ad88e45be"
/>

### 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.

<img width="740" height="212" alt="Screenshot 2026-05-06 at 6 24 43 PM"
src="https://github.com/user-attachments/assets/da9e9461-c352-4266-80b8-625a98e055ec"
/>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## 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.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
Nico
2026-05-07 16:10:15 +02:00
committed by GitHub
parent b47b1fc205
commit d8a1ffae81
4 changed files with 148 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files
changes/42637-clear-stale-broken-label-associations
+1
View File
@@ -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.
server/datastore/mysql/apple_mdm_test.go
+66
View File
@@ -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(`<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
server/datastore/mysql/mdm.go
+6 -3
View File
@@ -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 (?)
`
server/datastore/mysql/mdm_test.go
+75
View File
@@ -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) {