Fix GitOps failure when moving labels from global to fleet scope#44983
Conversation
) Use COALESCE(label_id, 0) in SELECT statements within batchSetProfileLabelAssociationsDB and batchSetDeclarationLabelAssociationsDB to handle NULL label_id values left by ON DELETE SET NULL when labels are deleted. Also add OR label_id IS NULL to the declaration labels DELETE statement to clean up broken rows, matching the profile labels behavior.
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
WalkthroughThis PR fixes a GitOps failure occurring when labels are moved from global to fleet scope. The root cause is stale association rows with NULL Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Fixes a GitOps failure when labels associated to MDM entities are deleted (leaving label_id = NULL via FK ON DELETE SET NULL) and then recreated with the same name, by ensuring datastore queries and cleanup logic handle NULL label_id values safely.
Changes:
- Use
COALESCE(label_id, 0)when selecting label associations to avoid scanningNULLinto non-nullableuintfields. - Expand association cleanup logic for declaration labels to also delete rows where
label_id IS NULL. - Add a regression test that reproduces the GitOps scenario (delete label → recreate same name → re-apply associations).
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| server/datastore/mysql/mdm_test.go | Adds a regression test covering the delete/recreate-same-name label scenario for profile label associations. |
| server/datastore/mysql/mdm.go | Updates the profile label association SELECT to coalesce NULL label_id values to 0. |
| server/datastore/mysql/apple_mdm.go | Updates declaration label association SELECT to coalesce NULL label_id values and ensures deletion cleans up NULL rows. |
| changes/44950-gitops-null-label-id | Adds a changelog entry describing the GitOps failure fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #44983 +/- ##
==========================================
+ Coverage 66.73% 66.75% +0.01%
==========================================
Files 2693 2694 +1
Lines 216780 217239 +459
Branches 10030 10030
==========================================
+ Hits 144677 145015 +338
- Misses 58969 59072 +103
- Partials 13134 13152 +18
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
) (#45003) Closes #44950 Reproduced locally using a MySQL integration test against the local test database. The test simulates the exact GitOps scenario from the issue: 1. Create a label and associate it with an MDM profile 2. Delete the label (FK `ON DELETE SET NULL` sets `label_id = NULL`) 3. Create a new label with the **same name** (simulates moving from global to fleet scope) 4. Call `batchSetProfileLabelAssociationsDB` with the profile referencing the new label **Before fix** (code from `main`, unfixed): ``` $ MYSQL_TEST=1 go test -run "TestMDMShared/TestBatchSetProfileLabelAssociations/same_label_name_recreated" -v -count=1 ./server/datastore/mysql/... === RUN TestMDMShared/TestBatchSetProfileLabelAssociations/same_label_name_recreated_after_deletion_darwin Error: selecting existing profile labels: sql: Scan error on column index 1, name "label_id": converting NULL to uint is unsupported === RUN TestMDMShared/TestBatchSetProfileLabelAssociations/same_label_name_recreated_after_deletion_windows Error: selecting existing profile labels: sql: Scan error on column index 1, name "label_id": converting NULL to uint is unsupported --- FAIL: TestMDMShared/TestBatchSetProfileLabelAssociations/same_label_name_recreated_after_deletion_darwin (0.02s) --- FAIL: TestMDMShared/TestBatchSetProfileLabelAssociations/same_label_name_recreated_after_deletion_windows (0.02s) FAIL ``` **After fix:** ``` $ MYSQL_TEST=1 go test -run "TestMDMShared/TestBatchSetProfileLabelAssociations/same_label_name_recreated" -v -count=1 ./server/datastore/mysql/... === RUN TestMDMShared/TestBatchSetProfileLabelAssociations/same_label_name_recreated_after_deletion_windows === RUN TestMDMShared/TestBatchSetProfileLabelAssociations/same_label_name_recreated_after_deletion_darwin --- PASS: TestMDMShared/TestBatchSetProfileLabelAssociations/same_label_name_recreated_after_deletion_windows (0.03s) --- PASS: TestMDMShared/TestBatchSetProfileLabelAssociations/same_label_name_recreated_after_deletion_darwin (0.03s) PASS ok github.com/fleetdm/fleet/v4/server/datastore/mysql 2.761s ``` When a label is deleted, MySQL's `ON DELETE SET NULL` foreign key constraint automatically sets `label_id = NULL` in the profile-label association row. The Go code then crashes trying to scan that NULL into a `uint` field. - **`server/datastore/mysql/mdm.go`** — Added `COALESCE(label_id, 0)` to the SELECT in `batchSetProfileLabelAssociationsDB`, so that NULL `label_id` values are returned as 0 instead of causing a scan error when Go tries to read NULL into a `uint`. - **`server/datastore/mysql/apple_mdm.go`** — Same `COALESCE(label_id, 0)` fix in `batchSetDeclarationLabelAssociationsDB`. Also added `OR label_id IS NULL` to the DELETE statement to clean up broken rows, matching the profile labels behavior from #42637. Other queries in the same codebase (e.g., `listProfileLabelsForProfiles`) already use `COALESCE(label_id, 0)` — these two were missed. - `same_label_name_recreated_after_deletion_{darwin,windows}` — reproduces the exact bug: associates a profile with a label, deletes the label (NULL label_id), creates a new label with the same name, and verifies `batchSetProfileLabelAssociationsDB` succeeds, the broken row is cleaned up, and the correct label association exists - Full MDM test suite passes: `MYSQL_TEST=1 go test -run "TestMDM" ./server/datastore/mysql/...` (76s) - `make lint-go-incremental` passes Co-authored-by: Sharon Katz <121527325+sharon-fdm@users.noreply.github.com>
Closes #44950
Local reproduction
Reproduced locally using a MySQL integration test against the local test database. The test simulates the exact GitOps scenario from the issue:
ON DELETE SET NULLsetslabel_id = NULL)batchSetProfileLabelAssociationsDBwith the profile referencing the new labelBefore fix (code from
main, unfixed):After fix:
Code changes
When a label is deleted, MySQL's
ON DELETE SET NULLforeign key constraint automatically setslabel_id = NULLin the profile-label association row. The Go code then crashes trying to scan that NULL into auintfield.server/datastore/mysql/mdm.go— AddedCOALESCE(label_id, 0)to the SELECT inbatchSetProfileLabelAssociationsDB, so that NULLlabel_idvalues are returned as 0 instead of causing a scan error when Go tries to read NULL into auint.server/datastore/mysql/apple_mdm.go— SameCOALESCE(label_id, 0)fix inbatchSetDeclarationLabelAssociationsDB. Also addedOR label_id IS NULLto the DELETE statement to clean up broken rows, matching the profile labels behavior fromfleetctl gitops applydoesn't clear stalelabels_exclude_anyassociations after label deletion #42637.Other queries in the same codebase (e.g.,
listProfileLabelsForProfiles) already useCOALESCE(label_id, 0)— these two were missed.Testing
same_label_name_recreated_after_deletion_{darwin,windows}— reproduces the exact bug: associates a profile with a label, deletes the label (NULL label_id), creates a new label with the same name, and verifiesbatchSetProfileLabelAssociationsDBsucceeds, the broken row is cleaned up, and the correct label association existsMYSQL_TEST=1 go test -run "TestMDM" ./server/datastore/mysql/...(76s)make lint-go-incrementalpasses