Fix fleet-scoped host vitals labels by lucasmrod · Pull Request #46953 · fleetdm/fleet

lucasmrod · 2026-06-05T20:21:58Z

Related issue: Resolves #46869

Changes file added for user-visible changes in changes/, orbit/changes/ or ee/fleetd-chrome/changes.

Testing

Added/updated automated tests
QA'd all new/changed functionality manually

Summary by CodeRabbit

Bug Fixes
- Host vitals labels based on identity-provider group membership now correctly apply to both global and team-scoped hosts, preventing cross-team leakage.
Tests
- Added and updated tests to validate IdP-group-backed vitals label membership across global and per-team hosts.

claude

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

coderabbitai · 2026-06-05T20:27:10Z

Need an answer fast? Review this PR in Change Stack to ask focused questions about the PR or a changed range.

Walkthrough

This PR fixes a bug where host vitals labels matching IdP groups were not capturing hosts when scoped to fleets or teams. The root cause involved two issues: the cron's label-discovery filter excluded fleet-scoped labels, and the SQL join logic used RIGHT JOIN instead of INNER JOIN, allowing NULL host IDs when hosts were filtered by team. The fix updates the SQL join to INNER JOIN, modifies the cron to use a global-admin TeamFilter for label discovery, and adds comprehensive test coverage at datastore and cron integration levels to verify correct host membership across global and team-scoped labels.

Possibly related PRs

fleetdm/fleet#45137: Related work ensuring host_scim_user mappings are created during MDM OTA ingest, which targets the same IdP/SCIM mapping data used by host-vitals population.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 10.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description check	✅ Passed	The PR description includes the related issue (`#46869`), a checked box confirming a changes file was added, and verification that automated tests were added/updated and manual QA was performed.
Linked Issues check	✅ Passed	The PR addresses the core issue `#46869` by fixing the SQL join in cronHostVitalsLabelMembership and hostForeignVitalGroups, adding comprehensive tests covering both global and team-scoped IdP labels, ensuring IdP-group-based labels now correctly capture hosts across team boundaries.
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing the IdP fleet-scoped labels issue: SQL join corrections, cron logic updates, test additions for IdP criteria, and query assertion updates in integration tests.
Title check	✅ Passed	The title 'Fix fleet-scoped host vitals labels' directly and clearly summarizes the main change: fixing a bug where fleet-scoped host vitals labels were not working correctly.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch 46869-fix-fleet-scoped-idp-labels

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Copilot

Pull request overview

Fixes fleet/team-scoped host vitals labels for IdP-based criteria (e.g. IdP group/department) by ensuring the membership cron includes team labels and by correcting the IdP foreign-vitals SQL join so out-of-scope hosts don’t survive filtering (and don’t break label membership updates).

Changes:

Change IdP foreign vitals join from RIGHT JOIN to an inner JOIN to prevent NULL host IDs and cross-team leakage during fleet-scoped label membership updates.
Update the host vitals label membership cron to list labels using a global-admin TeamFilter, so team-scoped host vitals labels are included.
Add/adjust regression tests (datastore + cron) and update expected generated SQL strings; add a user-visible changelog entry.

Reviewed changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
server/fleet/hosts.go	Switch IdP foreign-vitals query join semantics and document the rationale.
cmd/fleet/cron.go	Ensure cron lists labels with a global-admin filter so team-scoped host vitals labels are processed.
server/service/labels_test.go	Update expected generated SQL string for IdP vitals label query.
server/service/integration_core_test.go	Update expected generated SQL strings for IdP vitals label queries in integration tests.
server/datastore/mysql/labels_test.go	Add datastore regression test covering global vs team-scoped IdP host vitals labels.
cmd/fleet/serve_test.go	Strengthen unit test to assert cron uses a user-scoped/global-admin filter when listing labels.
cmd/fleet/cron_test.go	Add cron integration test ensuring team-scoped IdP host vitals labels get populated.
changes/46869-idp-team-vitals-labels	Changelog entry describing the bugfix.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

codecov · 2026-06-05T21:01:48Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 67.04%. Comparing base (ceb743d) to head (e7cd5b5).
⚠️ Report is 66 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #46953      +/-   ##
==========================================
+ Coverage   67.02%   67.04%   +0.01%     
==========================================
  Files        2859     2862       +3     
  Lines      224683   225040     +357     
  Branches    11614    11614              
==========================================
+ Hits       150592   150869     +277     
- Misses      60445    60501      +56     
- Partials    13646    13670      +24

Flag	Coverage Δ
backend	`68.75% <100.00%> (+0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Fix fleet-scoped idp labels

9a68391

lucasmrod requested a review from a team as a code owner June 5, 2026 20:21

Copilot AI review requested due to automatic review settings June 5, 2026 20:21

claude Bot reviewed Jun 5, 2026

View reviewed changes

lucasmrod temporarily deployed to Docker Hub June 5, 2026 20:22 — with GitHub Actions Inactive

Copilot started reviewing on behalf of lucasmrod June 5, 2026 20:22 View session

lucasmrod assigned juan-fdz-hawa Jun 5, 2026

Copilot AI reviewed Jun 5, 2026

View reviewed changes

Comment thread server/datastore/mysql/labels_test.go

Comment thread server/datastore/mysql/labels_test.go

Fix lint

32b3cfb

lucasmrod temporarily deployed to Docker Hub June 5, 2026 20:31 — with GitHub Actions Inactive

Fix invalid change

e7cd5b5

lucasmrod temporarily deployed to Docker Hub June 5, 2026 20:41 — with GitHub Actions Inactive

lucasmrod changed the title ~~Fix fleet-scoped idp labels~~ Fix fleet-scoped host vitals labels Jun 8, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix fleet-scoped host vitals labels#46953

Fix fleet-scoped host vitals labels#46953
lucasmrod wants to merge 3 commits into
mainfrom
46869-fix-fleet-scoped-idp-labels

lucasmrod commented Jun 5, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

claude Bot left a comment

Uh oh!

coderabbitai Bot commented Jun 5, 2026 •

edited

Loading

❌ Failed checks (1 warning)

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Jun 5, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

lucasmrod commented Jun 5, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Testing

Summary by CodeRabbit

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Claude Code Review

Uh oh!

coderabbitai Bot commented Jun 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Possibly related PRs

❌ Failed checks (1 warning)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Jun 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lucasmrod commented Jun 5, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 5, 2026 •

edited

Loading

codecov Bot commented Jun 5, 2026 •

edited

Loading