Policies: combine include/exclude label targeting + add labels_exclude_all (#33441)#47444
Open
nulmete wants to merge 4 commits into
Open
Policies: combine include/exclude label targeting + add labels_exclude_all (#33441)#47444nulmete wants to merge 4 commits into
nulmete wants to merge 4 commits into
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #47444 +/- ##
==========================================
- Coverage 67.20% 67.18% -0.03%
==========================================
Files 3350 3350
Lines 228159 228208 +49
Branches 11751 11751
==========================================
- Hits 153342 153316 -26
- Misses 60991 61046 +55
- Partials 13826 13846 +20
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…ude_all Allow policies to combine one include scope (labels_include_any/all) with one exclude scope (labels_exclude_any/all), and add the new labels_exclude_all option. Relaxes the previous one-of-three validation to one include + one exclude (rejecting two-include, two-exclude, and include/exclude overlap), persists all scopes, and adds the exclude_all gate to the host-targeting and membership-cleanup SQL. Renames the shared overlap helper ProfileLabelOverlap to a generic LabelOverlap. Premium-gated like labels_include_all; no DB migration needed. For #33441.
nulmete
force-pushed
the
33441-policies-custom-targets-backend
branch
from
f93536e to
967696f
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends policy label targeting to support combining one include scope with one exclude scope, adds the premium-only labels_exclude_all scope for policies, and centralizes include/exclude overlap detection into a shared fleet.LabelOverlap helper reused across policy and MDM/profile code paths.
Changes:
- Add
labels_exclude_allto policy API/request/response types, service handlers, datastore persistence, and host scoping queries. - Update policy label-scope validation rules to allow one include + one exclude scope, while still rejecting conflicting include/include and exclude/exclude combinations and preventing include/exclude overlaps.
- Replace the MDM-specific
ProfileLabelOverlaphelper with a sharedLabelOverlap, and update tests (unit, integration, datastore) for new semantics.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| server/service/windows_mdm_profiles.go | Switches overlap check to shared fleet.LabelOverlap. |
| server/service/team_policies.go | Plumbs LabelsExcludeAll through team policy create/modify paths and adjusts premium gating and label association checks. |
| server/service/mdm.go | Uses fleet.LabelOverlap for MDM profile include/exclude overlap validation. |
| server/service/integration_enterprise_test.go | Updates integration expectations for new policy label-scope rules (include+exclude allowed; include/include and exclude/exclude rejected). |
| server/service/global_policies.go | Plumbs LabelsExcludeAll through global policy create/spec apply paths and premium gating. |
| server/service/apple_mdm.go | Switches overlap check to shared fleet.LabelOverlap. |
| server/fleet/policies.go | Adds LabelsExcludeAll, splits conflicting-label errors into include vs exclude, and updates scope validation logic to allow include+exclude combinations and reject overlaps. |
| server/fleet/policies_test.go | Adds unit tests for the new policy label-scope validation rules. |
| server/fleet/mdm.go | Replaces ProfileLabelOverlap usage with LabelOverlap and removes the old helper. |
| server/fleet/labels.go | Introduces shared LabelOverlap utility. |
| server/fleet/labels_test.go | Adds tests for LabelOverlap. |
| server/fleet/api_policies.go | Adds labels_exclude_all to policy API request structs (premium-only). |
| server/datastore/mysql/policies.go | Persists/loads exclude_all scope and updates policy scoping SQL to account for exclude-all semantics. |
| server/datastore/mysql/policies_test.go | Updates datastore tests to reflect allowed include+exclude combinations and exercises exclude_all round-tripping and membership cleanup. |
| server/datastore/mysql/hosts.go | Updates ListPoliciesForHost scoping SQL to enforce exclude_all semantics. |
| cmd/fleetctl/fleetctl/gitops.go | Switches config profile overlap validation to shared fleet.LabelOverlap. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (4)
WalkthroughAdds a LabelsExcludeAll label-scope to policy API/models, verification, MySQL persistence, host-targeting SQL, service endpoints (with premium gating), and tests. Introduces a shared fleet.LabelOverlap helper used by MDM, GitOps, and OS-specific profile creation paths, replaces the old ProfileLabelOverlap, and updates tests to validate exclude-all semantics and conflict/error behavior. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
server/fleet/policies_test.go (1)
51-53: ⚡ Quick winAdd explicit cross-scope overlap cases to lock in validator intent.
Please add overlap tests for
include_any + exclude_allandinclude_all + exclude_anyusing the same label; these are core to the newly combined-scope behavior.Proposed test additions
{name: "overlap include_any/exclude_any", includeAny: []string{"a"}, excludeAny: []string{"a"}, wantErrContains: `label "a" cannot appear in both an include and an exclude list`}, {name: "overlap include_all/exclude_all", includeAll: []string{"a"}, excludeAll: []string{"a"}, wantErrContains: `label "a" cannot appear in both an include and an exclude list`}, + {name: "overlap include_any/exclude_all", includeAny: []string{"a"}, excludeAll: []string{"a"}, wantErrContains: `label "a" cannot appear in both an include and an exclude list`}, + {name: "overlap include_all/exclude_any", includeAll: []string{"a"}, excludeAny: []string{"a"}, wantErrContains: `label "a" cannot appear in both an include and an exclude list`}, }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/fleet/policies_test.go` around lines 51 - 53, Add two table-driven test cases to policies_test.go to assert cross-scope overlap validation: one named "overlap include_any/exclude_all" with includeAny: []string{"a"} and excludeAll: []string{"a"}, and one named "overlap include_all/exclude_any" with includeAll: []string{"a"} and excludeAny: []string{"a"}; both should expect wantErrContains: `label "a" cannot appear in both an include and an exclude list` so the combined-scope validator behavior is locked in (place these alongside the existing overlap cases in the test table).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@server/fleet/policies_test.go`:
- Around line 51-53: Add two table-driven test cases to policies_test.go to
assert cross-scope overlap validation: one named "overlap
include_any/exclude_all" with includeAny: []string{"a"} and excludeAll:
[]string{"a"}, and one named "overlap include_all/exclude_any" with includeAll:
[]string{"a"} and excludeAny: []string{"a"}; both should expect wantErrContains:
`label "a" cannot appear in both an include and an exclude list` so the
combined-scope validator behavior is locked in (place these alongside the
existing overlap cases in the test table).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1ce83527-fc8f-4be7-9ea1-787eb738938f
📒 Files selected for processing (16)
cmd/fleetctl/fleetctl/gitops.goserver/datastore/mysql/hosts.goserver/datastore/mysql/policies.goserver/datastore/mysql/policies_test.goserver/fleet/api_policies.goserver/fleet/labels.goserver/fleet/labels_test.goserver/fleet/mdm.goserver/fleet/policies.goserver/fleet/policies_test.goserver/service/apple_mdm.goserver/service/global_policies.goserver/service/integration_enterprise_test.goserver/service/mdm.goserver/service/team_policies.goserver/service/windows_mdm_profiles.go
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related issue: Resolves #46582
Checklist for submitter
changes/,orbit/changes/oree/fleetd-chrome/changes.See Changes files for more information.
(Already added as part of the frontend PRs which have been merged to main.)
Testing
Added/updated automated tests
QA'd all new/changed functionality manually
Screen.Recording.2026-06-11.at.3.48.18.PM.mov
Summary by CodeRabbit
New Features
Improvements
Tests