Skip to content

Commit

Permalink
[8.13] [RAM] add the must/must_not/should in the alert search strategy (
Browse files Browse the repository at this point in the history
#179775) (#179929)

# Backport

This will backport the following commits from `main` to `8.13`:
- [[RAM] add the must/must_not/should in the alert search strategy
(#179775)](#179775)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Xavier
Mouligneau","email":"xavier.mouligneau@elastic.co"},"sourceCommit":{"committedDate":"2024-04-03T13:08:51Z","message":"[RAM]
add the must/must_not/should in the alert search strategy
(#179775)\n\n## Summary\r\n\r\nWe realized that the filter from the
search bar is NOT using `must`,\r\n`must_not` and `should` in the
elastic search `query`. When we created\r\nour search strategy for
alerts, we only supported the `filter` parameter\r\nto make sure, we
won't by pass our rbac filter on top of the alerts. We\r\nthink that we
can open this different parameters now after few testing.\r\n\r\nWe
created API test integration to validate that we won't broken
our\r\nRBAC filter with these new params + I did few test in the dev
tools like\r\n```\r\nPUT /my-index\r\n{\r\n \"settings\": {\r\n
\"number_of_shards\": 1\r\n },\r\n \"mappings\": {\r\n \"properties\":
{\r\n \"pet\": { \"type\": \"text\" }\r\n }\r\n }\r\n}\r\n\r\nPOST
my-index/_doc/\r\n{\r\n \"pet\": \"cat\"\r\n}\r\n\r\nPOST
my-index/_doc/\r\n{\r\n \"pet\": \"dog\"\r\n}\r\n\r\nPOST
my-index/_doc/\r\n{\r\n \"pet\": \"bird\"\r\n}\r\n```\r\n\r\nThen, we
did few tests like\r\n```\r\n### MUST query\r\nPOST
test/_search\r\n{\r\n \"query\": {\r\n \"bool\": {\r\n \"filter\": [\r\n
{\r\n \"terms\": {\r\n \"pet\": [\r\n \"cat\"\r\n ]\r\n }\r\n }\r\n
],\r\n \"must\": [\r\n {\r\n \"terms\": {\r\n \"pet\": [\r\n
\"bird\"\r\n ]\r\n }\r\n }\r\n ]\r\n }\r\n }\r\n}\r\n\r\n###
Response\r\n{\r\n \"took\": 0,\r\n \"timed_out\": false,\r\n
\"_shards\": {\r\n \"total\": 1,\r\n \"successful\": 1,\r\n \"skipped\":
0,\r\n \"failed\": 0\r\n },\r\n \"hits\": {\r\n \"total\": {\r\n
\"value\": 0,\r\n \"relation\": \"eq\"\r\n },\r\n \"max_score\":
null,\r\n \"hits\": []\r\n }\r\n}\r\n```\r\n\r\n```\r\n### MUST_NOT
query\r\nPUT /test\r\n{\r\n \"settings\": {\r\n \"number_of_shards\":
1\r\n },\r\n \"mappings\": {\r\n \"properties\": {\r\n \"pet\": {
\"type\": \"text\" }\r\n }\r\n }\r\n}\r\n\r\nPOST test/_search\r\n{\r\n
\"query\": {\r\n \"bool\": {\r\n \"filter\": [\r\n {\r\n \"terms\":
{\r\n \"pet\": [\r\n \"cat\"\r\n ]\r\n }\r\n }\r\n ],\r\n \"must_not\":
[\r\n {\r\n \"terms\": {\r\n \"pet\": [\r\n \"cat\"\r\n ]\r\n }\r\n
}\r\n ]\r\n }\r\n }\r\n}\r\n\r\n### Response\r\n{\r\n \"took\": 0,\r\n
\"timed_out\": false,\r\n \"_shards\": {\r\n \"total\": 1,\r\n
\"successful\": 1,\r\n \"skipped\": 0,\r\n \"failed\": 0\r\n },\r\n
\"hits\": {\r\n \"total\": {\r\n \"value\": 0,\r\n \"relation\":
\"eq\"\r\n },\r\n \"max_score\": null,\r\n \"hits\": []\r\n
}\r\n}\r\n```\r\n\r\n\r\n```\r\n### SHOULD query\r\nPOST
test/_search\r\n{\r\n \"query\": {\r\n \"bool\": {\r\n \"filter\": [\r\n
{\r\n \"terms\": {\r\n \"pet\": [\r\n \"cat\"\r\n ]\r\n }\r\n }\r\n
],\r\n \"should\": [\r\n {\r\n \"terms\": {\r\n \"pet\": [\r\n
\"bird\"\r\n ]\r\n }\r\n }\r\n ]\r\n }\r\n }\r\n}\r\n\r\n###
Response\r\n{\r\n \"took\": 0,\r\n \"timed_out\": false,\r\n
\"_shards\": {\r\n \"total\": 1,\r\n \"successful\": 1,\r\n \"skipped\":
0,\r\n \"failed\": 0\r\n },\r\n \"hits\": {\r\n \"total\": {\r\n
\"value\": 1,\r\n \"relation\": \"eq\"\r\n },\r\n \"max_score\": 0,\r\n
\"hits\": [\r\n {\r\n \"_index\": \"test\",\r\n \"_id\": \"1\",\r\n
\"_score\": 0,\r\n \"_source\": {\r\n \"pet\": \"cat\"\r\n }\r\n }\r\n
]\r\n }\r\n}\r\n```\r\n\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"538a10b85de66ccf08371ba89c01cf9e7495f7ba","branchLabelMapping":{"^v8.14.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:fix","Team:ResponseOps","v8.14.0","v8.13.1"],"title":"[RAM]
add the must/must_not/should in the alert search
strategy","number":179775,"url":"https://github.com/elastic/kibana/pull/179775","mergeCommit":{"message":"[RAM]
add the must/must_not/should in the alert search strategy
(#179775)\n\n## Summary\r\n\r\nWe realized that the filter from the
search bar is NOT using `must`,\r\n`must_not` and `should` in the
elastic search `query`. When we created\r\nour search strategy for
alerts, we only supported the `filter` parameter\r\nto make sure, we
won't by pass our rbac filter on top of the alerts. We\r\nthink that we
can open this different parameters now after few testing.\r\n\r\nWe
created API test integration to validate that we won't broken
our\r\nRBAC filter with these new params + I did few test in the dev
tools like\r\n```\r\nPUT /my-index\r\n{\r\n \"settings\": {\r\n
\"number_of_shards\": 1\r\n },\r\n \"mappings\": {\r\n \"properties\":
{\r\n \"pet\": { \"type\": \"text\" }\r\n }\r\n }\r\n}\r\n\r\nPOST
my-index/_doc/\r\n{\r\n \"pet\": \"cat\"\r\n}\r\n\r\nPOST
my-index/_doc/\r\n{\r\n \"pet\": \"dog\"\r\n}\r\n\r\nPOST
my-index/_doc/\r\n{\r\n \"pet\": \"bird\"\r\n}\r\n```\r\n\r\nThen, we
did few tests like\r\n```\r\n### MUST query\r\nPOST
test/_search\r\n{\r\n \"query\": {\r\n \"bool\": {\r\n \"filter\": [\r\n
{\r\n \"terms\": {\r\n \"pet\": [\r\n \"cat\"\r\n ]\r\n }\r\n }\r\n
],\r\n \"must\": [\r\n {\r\n \"terms\": {\r\n \"pet\": [\r\n
\"bird\"\r\n ]\r\n }\r\n }\r\n ]\r\n }\r\n }\r\n}\r\n\r\n###
Response\r\n{\r\n \"took\": 0,\r\n \"timed_out\": false,\r\n
\"_shards\": {\r\n \"total\": 1,\r\n \"successful\": 1,\r\n \"skipped\":
0,\r\n \"failed\": 0\r\n },\r\n \"hits\": {\r\n \"total\": {\r\n
\"value\": 0,\r\n \"relation\": \"eq\"\r\n },\r\n \"max_score\":
null,\r\n \"hits\": []\r\n }\r\n}\r\n```\r\n\r\n```\r\n### MUST_NOT
query\r\nPUT /test\r\n{\r\n \"settings\": {\r\n \"number_of_shards\":
1\r\n },\r\n \"mappings\": {\r\n \"properties\": {\r\n \"pet\": {
\"type\": \"text\" }\r\n }\r\n }\r\n}\r\n\r\nPOST test/_search\r\n{\r\n
\"query\": {\r\n \"bool\": {\r\n \"filter\": [\r\n {\r\n \"terms\":
{\r\n \"pet\": [\r\n \"cat\"\r\n ]\r\n }\r\n }\r\n ],\r\n \"must_not\":
[\r\n {\r\n \"terms\": {\r\n \"pet\": [\r\n \"cat\"\r\n ]\r\n }\r\n
}\r\n ]\r\n }\r\n }\r\n}\r\n\r\n### Response\r\n{\r\n \"took\": 0,\r\n
\"timed_out\": false,\r\n \"_shards\": {\r\n \"total\": 1,\r\n
\"successful\": 1,\r\n \"skipped\": 0,\r\n \"failed\": 0\r\n },\r\n
\"hits\": {\r\n \"total\": {\r\n \"value\": 0,\r\n \"relation\":
\"eq\"\r\n },\r\n \"max_score\": null,\r\n \"hits\": []\r\n
}\r\n}\r\n```\r\n\r\n\r\n```\r\n### SHOULD query\r\nPOST
test/_search\r\n{\r\n \"query\": {\r\n \"bool\": {\r\n \"filter\": [\r\n
{\r\n \"terms\": {\r\n \"pet\": [\r\n \"cat\"\r\n ]\r\n }\r\n }\r\n
],\r\n \"should\": [\r\n {\r\n \"terms\": {\r\n \"pet\": [\r\n
\"bird\"\r\n ]\r\n }\r\n }\r\n ]\r\n }\r\n }\r\n}\r\n\r\n###
Response\r\n{\r\n \"took\": 0,\r\n \"timed_out\": false,\r\n
\"_shards\": {\r\n \"total\": 1,\r\n \"successful\": 1,\r\n \"skipped\":
0,\r\n \"failed\": 0\r\n },\r\n \"hits\": {\r\n \"total\": {\r\n
\"value\": 1,\r\n \"relation\": \"eq\"\r\n },\r\n \"max_score\": 0,\r\n
\"hits\": [\r\n {\r\n \"_index\": \"test\",\r\n \"_id\": \"1\",\r\n
\"_score\": 0,\r\n \"_source\": {\r\n \"pet\": \"cat\"\r\n }\r\n }\r\n
]\r\n }\r\n}\r\n```\r\n\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"538a10b85de66ccf08371ba89c01cf9e7495f7ba"}},"sourceBranch":"main","suggestedTargetBranches":["8.13"],"targetPullRequestStates":[{"branch":"main","label":"v8.14.0","branchLabelMappingKey":"^v8.14.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/179775","number":179775,"mergeCommit":{"message":"[RAM]
add the must/must_not/should in the alert search strategy
(#179775)\n\n## Summary\r\n\r\nWe realized that the filter from the
search bar is NOT using `must`,\r\n`must_not` and `should` in the
elastic search `query`. When we created\r\nour search strategy for
alerts, we only supported the `filter` parameter\r\nto make sure, we
won't by pass our rbac filter on top of the alerts. We\r\nthink that we
can open this different parameters now after few testing.\r\n\r\nWe
created API test integration to validate that we won't broken
our\r\nRBAC filter with these new params + I did few test in the dev
tools like\r\n```\r\nPUT /my-index\r\n{\r\n \"settings\": {\r\n
\"number_of_shards\": 1\r\n },\r\n \"mappings\": {\r\n \"properties\":
{\r\n \"pet\": { \"type\": \"text\" }\r\n }\r\n }\r\n}\r\n\r\nPOST
my-index/_doc/\r\n{\r\n \"pet\": \"cat\"\r\n}\r\n\r\nPOST
my-index/_doc/\r\n{\r\n \"pet\": \"dog\"\r\n}\r\n\r\nPOST
my-index/_doc/\r\n{\r\n \"pet\": \"bird\"\r\n}\r\n```\r\n\r\nThen, we
did few tests like\r\n```\r\n### MUST query\r\nPOST
test/_search\r\n{\r\n \"query\": {\r\n \"bool\": {\r\n \"filter\": [\r\n
{\r\n \"terms\": {\r\n \"pet\": [\r\n \"cat\"\r\n ]\r\n }\r\n }\r\n
],\r\n \"must\": [\r\n {\r\n \"terms\": {\r\n \"pet\": [\r\n
\"bird\"\r\n ]\r\n }\r\n }\r\n ]\r\n }\r\n }\r\n}\r\n\r\n###
Response\r\n{\r\n \"took\": 0,\r\n \"timed_out\": false,\r\n
\"_shards\": {\r\n \"total\": 1,\r\n \"successful\": 1,\r\n \"skipped\":
0,\r\n \"failed\": 0\r\n },\r\n \"hits\": {\r\n \"total\": {\r\n
\"value\": 0,\r\n \"relation\": \"eq\"\r\n },\r\n \"max_score\":
null,\r\n \"hits\": []\r\n }\r\n}\r\n```\r\n\r\n```\r\n### MUST_NOT
query\r\nPUT /test\r\n{\r\n \"settings\": {\r\n \"number_of_shards\":
1\r\n },\r\n \"mappings\": {\r\n \"properties\": {\r\n \"pet\": {
\"type\": \"text\" }\r\n }\r\n }\r\n}\r\n\r\nPOST test/_search\r\n{\r\n
\"query\": {\r\n \"bool\": {\r\n \"filter\": [\r\n {\r\n \"terms\":
{\r\n \"pet\": [\r\n \"cat\"\r\n ]\r\n }\r\n }\r\n ],\r\n \"must_not\":
[\r\n {\r\n \"terms\": {\r\n \"pet\": [\r\n \"cat\"\r\n ]\r\n }\r\n
}\r\n ]\r\n }\r\n }\r\n}\r\n\r\n### Response\r\n{\r\n \"took\": 0,\r\n
\"timed_out\": false,\r\n \"_shards\": {\r\n \"total\": 1,\r\n
\"successful\": 1,\r\n \"skipped\": 0,\r\n \"failed\": 0\r\n },\r\n
\"hits\": {\r\n \"total\": {\r\n \"value\": 0,\r\n \"relation\":
\"eq\"\r\n },\r\n \"max_score\": null,\r\n \"hits\": []\r\n
}\r\n}\r\n```\r\n\r\n\r\n```\r\n### SHOULD query\r\nPOST
test/_search\r\n{\r\n \"query\": {\r\n \"bool\": {\r\n \"filter\": [\r\n
{\r\n \"terms\": {\r\n \"pet\": [\r\n \"cat\"\r\n ]\r\n }\r\n }\r\n
],\r\n \"should\": [\r\n {\r\n \"terms\": {\r\n \"pet\": [\r\n
\"bird\"\r\n ]\r\n }\r\n }\r\n ]\r\n }\r\n }\r\n}\r\n\r\n###
Response\r\n{\r\n \"took\": 0,\r\n \"timed_out\": false,\r\n
\"_shards\": {\r\n \"total\": 1,\r\n \"successful\": 1,\r\n \"skipped\":
0,\r\n \"failed\": 0\r\n },\r\n \"hits\": {\r\n \"total\": {\r\n
\"value\": 1,\r\n \"relation\": \"eq\"\r\n },\r\n \"max_score\": 0,\r\n
\"hits\": [\r\n {\r\n \"_index\": \"test\",\r\n \"_id\": \"1\",\r\n
\"_score\": 0,\r\n \"_source\": {\r\n \"pet\": \"cat\"\r\n }\r\n }\r\n
]\r\n }\r\n}\r\n```\r\n\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"538a10b85de66ccf08371ba89c01cf9e7495f7ba"}},{"branch":"8.13","label":"v8.13.1","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Xavier Mouligneau <xavier.mouligneau@elastic.co>
  • Loading branch information
kibanamachine and XavierM authored Apr 3, 2024
1 parent dd3e76c commit 13fb5dd
Show file tree
Hide file tree
Showing 2 changed files with 129 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ export const ruleRegistrySearchStrategyProvider = (
? { ids: request.query?.ids }
: {
bool: {
...request.query?.bool,
filter,
},
}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,134 @@ export default ({ getService }: FtrProviderContext) => {
);
expect(consumers.every((consumer) => consumer === AlertConsumers.APM));
});

it('should not by pass our RBAC authz filter with a should filter', async () => {
const result = await secureBsearch.send<RuleRegistrySearchResponse>({
supertestWithoutAuth,
auth: {
username: obsOnlySpacesAll.username,
password: obsOnlySpacesAll.password,
},
referer: 'test',
kibanaVersion,
internalOrigin: 'Kibana',
options: {
featureIds: [AlertConsumers.APM],
query: {
bool: {
filter: [],
should: [
{
bool: {
should: [
{
match: {
'kibana.alert.rule.consumer': 'logs',
},
},
],
minimum_should_match: 1,
},
},
],
must: [],
must_not: [],
},
},
},
strategy: 'privateRuleRegistryAlertsSearchStrategy',
space: 'default',
});
expect(result.rawResponse.hits.total).to.eql(9);
const consumers = result.rawResponse.hits.hits.map(
(hit) => hit.fields?.['kibana.alert.rule.consumer']
);
expect(consumers.every((consumer) => consumer === AlertConsumers.APM));
});

it('should return an empty response with must filter and our RBAC authz filter', async () => {
const result = await secureBsearch.send<RuleRegistrySearchResponse>({
supertestWithoutAuth,
auth: {
username: obsOnlySpacesAll.username,
password: obsOnlySpacesAll.password,
},
referer: 'test',
kibanaVersion,
internalOrigin: 'Kibana',
options: {
featureIds: [AlertConsumers.APM],
query: {
bool: {
filter: [],
must: [
{
bool: {
should: [
{
match: {
'kibana.alert.rule.consumer': 'logs',
},
},
],
minimum_should_match: 1,
},
},
],
should: [],
must_not: [],
},
},
},
strategy: 'privateRuleRegistryAlertsSearchStrategy',
space: 'default',
});
expect(result.rawResponse.hits.total).to.eql(0);
});

it('should not by pass our RBAC authz filter with must_not filter', async () => {
const result = await secureBsearch.send<RuleRegistrySearchResponse>({
supertestWithoutAuth,
auth: {
username: obsOnlySpacesAll.username,
password: obsOnlySpacesAll.password,
},
referer: 'test',
kibanaVersion,
internalOrigin: 'Kibana',
options: {
featureIds: [AlertConsumers.APM],
query: {
bool: {
filter: [],
must: [],
must_not: [
{
bool: {
should: [
{
match: {
'kibana.alert.rule.consumer': 'apm',
},
},
],
minimum_should_match: 1,
},
},
],
should: [],
},
},
},
strategy: 'privateRuleRegistryAlertsSearchStrategy',
space: 'default',
});
expect(result.rawResponse.hits.total).to.eql(9);
const consumers = result.rawResponse.hits.hits.map(
(hit) => hit.fields?.['kibana.alert.rule.consumer']
);
expect(consumers.every((consumer) => consumer === AlertConsumers.APM));
});
});

describe('empty response', () => {
Expand Down

0 comments on commit 13fb5dd

Please sign in to comment.