Added license checks when targeting queries by label (33045)#41255
Merged
juan-fdz-hawa merged 2 commits intomainfrom Mar 9, 2026
Merged
Conversation
When creating or updating queries either via the API or Gitops, make sure that the license is premium if the query is targeting a label.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #41255 +/- ##
=======================================
Coverage 66.34% 66.35%
=======================================
Files 2477 2477
Lines 198439 198470 +31
Branches 8890 8890
=======================================
+ Hits 131660 131700 +40
+ Misses 54876 54865 -11
- Partials 11903 11905 +2
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:
|
Contributor
Author
|
@coderabbitai full review |
Contributor
✅ Actions performedFull review triggered. |
Contributor
WalkthroughThis PR adds license enforcement for label-based query targeting across the Fleet REST API and CLI. It introduces premium license checks in the query creation ( 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 unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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.
Actionable comments posted: 1
🧹 Nitpick comments (2)
cmd/fleetctl/fleetctl/gitops_test.go (2)
1162-1164: Tighten this label-ID mock so it can't hide partial resolution bugs.Right now the stub returns both IDs no matter what
namescontains. That means this test would still pass if GitOps only asked the datastore for one label, or the wrong set entirely.Suggested tightening
ds.LabelIDsByNameFunc = func(ctx context.Context, names []string, filter fleet.TeamFilter) (map[string]uint, error) { - return map[string]uint{"a": 1, "b": 2}, nil + require.ElementsMatch(t, []string{"a", "b"}, names) + ids := make(map[string]uint, len(names)) + for _, name := range names { + switch name { + case "a": + ids[name] = 1 + case "b": + ids[name] = 2 + } + } + return ids, nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/fleetctl/fleetctl/gitops_test.go` around lines 1162 - 1164, The mock for LabelIDsByNameFunc is too permissive and returns both IDs regardless of the input, which can hide bugs where the code requests the wrong labels or a subset; update the LabelIDsByNameFunc stub to inspect the incoming names slice and return a map containing only the requested label names (or an error for unknown names) so tests fail when the code queries the wrong set—change the implementation of LabelIDsByNameFunc to validate the names argument and produce a result keyed only by those names (or return an error) instead of always returning {"a":1,"b":2}.
1247-1249: Avoid keying this assertion offappliedQueries[0].These checks are order-dependent. If
ApplyQuerieschanges ordering, this test will fail even though label resolution is still correct. Resolve the query by name first, then assert its labels.Suggested rewrite
- assert.Len(t, appliedQueries[0].LabelsIncludeAny, 2) - assert.Contains(t, []string{appliedQueries[0].LabelsIncludeAny[0].LabelName, appliedQueries[0].LabelsIncludeAny[1].LabelName}, "a") - assert.Contains(t, []string{appliedQueries[0].LabelsIncludeAny[0].LabelName, appliedQueries[0].LabelsIncludeAny[1].LabelName}, "b") + var labeledQuery *fleet.Query + for _, q := range appliedQueries { + if q.Name == "test_query" || q.Name == "test-query" { + labeledQuery = q + break + } + } + require.NotNil(t, labeledQuery) + require.Len(t, labeledQuery.LabelsIncludeAny, 2) + assert.ElementsMatch(t, []string{"a", "b"}, []string{ + labeledQuery.LabelsIncludeAny[0].LabelName, + labeledQuery.LabelsIncludeAny[1].LabelName, + })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/fleetctl/fleetctl/gitops_test.go` around lines 1247 - 1249, Test currently asserts labels using appliedQueries[0], which is order-dependent; instead locate the specific query by its name within the appliedQueries slice (e.g., find the element whose Name or QueryName equals the expected value) and then assert against that query's LabelsIncludeAny values in an order-independent way (verify the set contains "a" and "b" by checking the LabelName values of LabelsIncludeAny). Use the foundQuery variable (or similar) rather than appliedQueries[0] and assert membership of both labels without relying on element order.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/fleetctl/fleetctl/gitops.go`:
- Around line 315-322: Fix the typo in the inline comment: change "Targeting
queries agaisnt labels is a Premium feature only" to "Targeting queries against
labels is a Premium feature only" near the block that checks
appConfig.License.IsPremium() and iterates over config.Queries to inspect
query.LabelsIncludeAny; do not modify the conditional logic checking
len(query.LabelsIncludeAny) > 0 or the error message returned.
---
Nitpick comments:
In `@cmd/fleetctl/fleetctl/gitops_test.go`:
- Around line 1162-1164: The mock for LabelIDsByNameFunc is too permissive and
returns both IDs regardless of the input, which can hide bugs where the code
requests the wrong labels or a subset; update the LabelIDsByNameFunc stub to
inspect the incoming names slice and return a map containing only the requested
label names (or an error for unknown names) so tests fail when the code queries
the wrong set—change the implementation of LabelIDsByNameFunc to validate the
names argument and produce a result keyed only by those names (or return an
error) instead of always returning {"a":1,"b":2}.
- Around line 1247-1249: Test currently asserts labels using appliedQueries[0],
which is order-dependent; instead locate the specific query by its name within
the appliedQueries slice (e.g., find the element whose Name or QueryName equals
the expected value) and then assert against that query's LabelsIncludeAny values
in an order-independent way (verify the set contains "a" and "b" by checking the
LabelName values of LabelsIncludeAny). Use the foundQuery variable (or similar)
rather than appliedQueries[0] and assert membership of both labels without
relying on element order.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4784d944-8847-45cd-9ead-512c37ec5337
📒 Files selected for processing (6)
changes/33045-fleet-free-targetting-queries-by-labelcmd/fleetctl/fleetctl/gitops.gocmd/fleetctl/fleetctl/gitops_test.goserver/service/integration_core_test.goserver/service/queries.goserver/service/queries_test.go
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
sgress454
approved these changes
Mar 9, 2026
| return document, nil, nil | ||
| } | ||
|
|
||
| // Premium mocks |
Contributor
There was a problem hiding this comment.
only the freshest, hottest mocks
juan-fdz-hawa
deleted the
33045-fleet-free---targeting-queries-by-label-not-blocked-rest-api
branch
This was referenced Apr 29, 2026
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
2 participants
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 #33045
When creating or updating queries either via the API or Gitops, make sure that the license is premium if the query is targeting a label.
Checklist for submitter
If some of the following don't apply, delete the relevant line.
changes/,orbit/changes/oree/fleetd-chrome/changes.See Changes files for more information.
Testing
Summary by CodeRabbit
Release Notes