Skip to content

Commit

Permalink
[8.x] [Security Solution] Fix `DataSource` payload creation…
Browse files Browse the repository at this point in the history
… during rule upgrade with `MERGED` pick_version (#197262) (#197467)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Security Solution] Fix `DataSource` payload creation
during rule upgrade with `MERGED` pick_version
(#197262)](#197262)

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

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

<!--BACKPORT [{"author":{"name":"Juan Pablo
Djeredjian","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-23T14:44:13Z","message":"[Security
Solution] Fix `DataSource` payload creation during rule upgrade with
`MERGED` pick_version (#197262)\n\n## Summary\r\n\r\nThe PR
#191439 enhanced
the\r\n`/upgrade/_perform` API contract and functionality to allow the
users of\r\nthe endpoint to upgrade rules to their `MERGED`
version.\r\n\r\nHowever, a bug slipped in, where the two different types
of `DataSource`\r\n(`type: index_patterns` or `type: data_view_id`)
weren't properly\r\nhandled and would cause, in some cases, a rule
payload to be created\r\nhaving both an `index` and `data_view` field,
causing upgrade to fail.\r\n\r\nThis PR fixes the issue by handling
these two field in a specific way,\r\nchecking what the `DataSource`
diffable field's type is, and setting the\r\nother field to
`undefined`.\r\n\r\n\r\n### Checklist\r\n\r\nDelete any items that are
not applicable to this PR.\r\n\r\n- [ ] [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\r\n- [ ] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n\r\n\r\n\r\n### For
maintainers\r\n\r\n- [ ] This was checked for breaking API changes and
was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)\r\n-
[ ] This will appear in the **Release Notes** and follow
the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"9656621fcc8f6f9a615b0a27d45db9722e047a10","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","Team:Detections
and Resp","Team: SecuritySolution","Team:Detection Rule
Management","Feature:Prebuilt Detection
Rules","v8.16.0","backport:version","v8.17.0"],"title":"[Security
Solution] Fix `DataSource` payload creation during rule upgrade with
`MERGED`
pick_version","number":197262,"url":"https://github.com/elastic/kibana/pull/197262","mergeCommit":{"message":"[Security
Solution] Fix `DataSource` payload creation during rule upgrade with
`MERGED` pick_version (#197262)\n\n## Summary\r\n\r\nThe PR
#191439 enhanced
the\r\n`/upgrade/_perform` API contract and functionality to allow the
users of\r\nthe endpoint to upgrade rules to their `MERGED`
version.\r\n\r\nHowever, a bug slipped in, where the two different types
of `DataSource`\r\n(`type: index_patterns` or `type: data_view_id`)
weren't properly\r\nhandled and would cause, in some cases, a rule
payload to be created\r\nhaving both an `index` and `data_view` field,
causing upgrade to fail.\r\n\r\nThis PR fixes the issue by handling
these two field in a specific way,\r\nchecking what the `DataSource`
diffable field's type is, and setting the\r\nother field to
`undefined`.\r\n\r\n\r\n### Checklist\r\n\r\nDelete any items that are
not applicable to this PR.\r\n\r\n- [ ] [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\r\n- [ ] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n\r\n\r\n\r\n### For
maintainers\r\n\r\n- [ ] This was checked for breaking API changes and
was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)\r\n-
[ ] This will appear in the **Release Notes** and follow
the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"9656621fcc8f6f9a615b0a27d45db9722e047a10"}},"sourceBranch":"main","suggestedTargetBranches":["8.16","8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/197262","number":197262,"mergeCommit":{"message":"[Security
Solution] Fix `DataSource` payload creation during rule upgrade with
`MERGED` pick_version (#197262)\n\n## Summary\r\n\r\nThe PR
#191439 enhanced
the\r\n`/upgrade/_perform` API contract and functionality to allow the
users of\r\nthe endpoint to upgrade rules to their `MERGED`
version.\r\n\r\nHowever, a bug slipped in, where the two different types
of `DataSource`\r\n(`type: index_patterns` or `type: data_view_id`)
weren't properly\r\nhandled and would cause, in some cases, a rule
payload to be created\r\nhaving both an `index` and `data_view` field,
causing upgrade to fail.\r\n\r\nThis PR fixes the issue by handling
these two field in a specific way,\r\nchecking what the `DataSource`
diffable field's type is, and setting the\r\nother field to
`undefined`.\r\n\r\n\r\n### Checklist\r\n\r\nDelete any items that are
not applicable to this PR.\r\n\r\n- [ ] [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\r\n- [ ] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n\r\n\r\n\r\n### For
maintainers\r\n\r\n- [ ] This was checked for breaking API changes and
was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)\r\n-
[ ] This will appear in the **Release Notes** and follow
the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"9656621fcc8f6f9a615b0a27d45db9722e047a10"}},{"branch":"8.16","label":"v8.16.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.x","label":"v8.17.0","branchLabelMappingKey":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Juan Pablo Djeredjian <[email protected]>
  • Loading branch information
kibanamachine and jpdjere authored Oct 23, 2024
1 parent d95f16d commit e9dcb30
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
import { get } from 'lodash';
import type {
RuleSchedule,
DataSourceIndexPatterns,
DataSourceDataView,
InlineKqlQuery,
ThreeWayDiff,
DiffableRuleTypes,
Expand Down Expand Up @@ -195,6 +197,10 @@ export const transformDiffableFieldValues = (
} else if (fieldName === 'saved_id' && isInlineQuery(diffableFieldValue)) {
// saved_id should be set only for rules with SavedKqlQuery, undefined otherwise
return { type: 'TRANSFORMED_FIELD', value: undefined };
} else if (fieldName === 'data_view_id' && isDataSourceIndexPatterns(diffableFieldValue)) {
return { type: 'TRANSFORMED_FIELD', value: undefined };
} else if (fieldName === 'index' && isDataSourceDataView(diffableFieldValue)) {
return { type: 'TRANSFORMED_FIELD', value: undefined };
}

return { type: 'NON_TRANSFORMED_FIELD' };
Expand All @@ -209,3 +215,18 @@ function isInlineQuery(value: unknown): value is InlineKqlQuery {
typeof value === 'object' && value !== null && 'type' in value && value.type === 'inline_query'
);
}

function isDataSourceIndexPatterns(value: unknown): value is DataSourceIndexPatterns {
return (
typeof value === 'object' &&
value !== null &&
'type' in value &&
value.type === 'index_patterns'
);
}

function isDataSourceDataView(value: unknown): value is DataSourceDataView {
return (
typeof value === 'object' && value !== null && 'type' in value && value.type === 'data_view'
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ import {
ThreatMatchRule,
FIELDS_TO_UPGRADE_TO_CURRENT_VERSION,
ModeEnum,
AllFieldsDiff,
DataSourceIndexPatterns,
QueryRule,
} from '@kbn/security-solution-plugin/common/api/detection_engine';
import { PrebuiltRuleAsset } from '@kbn/security-solution-plugin/server/lib/detection_engine/prebuilt_rules';
import { FtrProviderContext } from '../../../../../../ftr_provider_context';
Expand Down Expand Up @@ -246,6 +249,41 @@ export default ({ getService }: FtrProviderContext): void => {
expect(installedRule.tags).toEqual(reviewRuleResponseMap.get(ruleId)?.tags);
}
});

it('correctly upgrades rules with DataSource diffs to their MERGED versions', async () => {
await createHistoricalPrebuiltRuleAssetSavedObjects(es, [queryRule]);
await installPrebuiltRules(es, supertest);

const targetObject = cloneDeep(queryRule);
targetObject['security-rule'].version += 1;
targetObject['security-rule'].name = TARGET_NAME;
targetObject['security-rule'].tags = TARGET_TAGS;
targetObject['security-rule'].index = ['auditbeat-*'];
await createHistoricalPrebuiltRuleAssetSavedObjects(es, [targetObject]);

const reviewResponse = await reviewPrebuiltRulesToUpgrade(supertest);
const ruleDiffFields = reviewResponse.rules[0].diff.fields as AllFieldsDiff;

const performUpgradeResponse = await performUpgradePrebuiltRules(es, supertest, {
mode: ModeEnum.ALL_RULES,
pick_version: 'MERGED',
});

expect(performUpgradeResponse.summary.succeeded).toEqual(1);

const installedRules = await getInstalledRules(supertest);
const installedRule = installedRules.data[0] as QueryRule;

expect(installedRule.name).toEqual(ruleDiffFields.name.merged_version);
expect(installedRule.tags).toEqual(ruleDiffFields.tags.merged_version);

// Check that the updated rules has an `index` field which equals the output of the diff algorithm
// for the DataSource diffable field, and that the data_view_id is correspondingly set to undefined.
expect(installedRule.index).toEqual(
(ruleDiffFields.data_source.merged_version as DataSourceIndexPatterns).index_patterns
);
expect(installedRule.data_view_id).toBe(undefined);
});
});

describe('edge cases and unhappy paths', () => {
Expand Down

0 comments on commit e9dcb30

Please sign in to comment.