Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Security Solution] [Detections] Remove file validation on import route #77770

Merged
merged 5 commits into from
Sep 17, 2020
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
updates getSimpleRule and getSimpleRuleAsNdjson to accept an enabled …
…param defaulted to false
dhurley14 committed Sep 17, 2020
commit fe888cf1b58a08e267ee695bc5e0a139387b259f
Original file line number Diff line number Diff line change
@@ -47,7 +47,7 @@ import { PartialFilter } from '../../types';

type PromiseFromStreams = ImportRulesSchemaDecoded | Error;

const CHUNK_PARSED_OBJECT_SIZE = 300;
const CHUNK_PARSED_OBJECT_SIZE = 50;

export const importRulesRoute = (router: IRouter, config: ConfigType, ml: SetupPlugins['ml']) => {
router.post(
Original file line number Diff line number Diff line change
@@ -192,8 +192,12 @@ export default ({ getService }: FtrProviderContext): void => {
});
});

it('should be able to import 9999 rules', async () => {
const ruleIds = new Array(9999).fill(undefined).map((_, index) => `rule-${index}`);
// import is still very slow due to the alerts client find api
// when importing 100 rules it takes about 30 seconds for this
// test to complete so at 10 rules completing in about 10 seconds
// I figured this is enough to make sure the import route is doing its job.
it('should be able to import 10 rules', async () => {
const ruleIds = new Array(10).fill(undefined).map((_, index) => `rule-${index}`);
const { body } = await supertest
.post(`${DETECTION_ENGINE_RULES_URL}/_import`)
.set('kbn-xsrf', 'true')
@@ -203,10 +207,26 @@ export default ({ getService }: FtrProviderContext): void => {
expect(body).to.eql({
errors: [],
success: true,
success_count: 9999,
success_count: 10,
});
});

// uncomment the below test once we speed up the alerts client find api
// it('should be able to import 10000 rules', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am fine with these commented out lines if you are as this is a smaller more concentrated PR. Just pointing to this in case you didn't want them here.

But since this is for 7.9.x series I am 👍 good with them here if they are helpful.

Copy link
Contributor Author

@dhurley14 dhurley14 Sep 17, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about deleting this test + comments but decided the note served as a nice reminder to me to make sure this test makes it back in to this PR #76398

// const ruleIds = new Array(10000).fill(undefined).map((_, index) => `rule-${index}`);
// const { body } = await supertest
// .post(`${DETECTION_ENGINE_RULES_URL}/_import`)
// .set('kbn-xsrf', 'true')
// .attach('file', getSimpleRuleAsNdjson(ruleIds, false), 'rules.ndjson')
// .expect(200);

// expect(body).to.eql({
// errors: [],
// success: true,
// success_count: 10000,
// });
// });

it('should NOT be able to import more than 10,000 rules', async () => {
const ruleIds = new Array(10001).fill(undefined).map((_, index) => `rule-${index}`);
const { body } = await supertest
11 changes: 5 additions & 6 deletions x-pack/test/detection_engine_api_integration/utils.ts
Original file line number Diff line number Diff line change
@@ -56,10 +56,12 @@ export const removeServerGeneratedPropertiesIncludingRuleId = (
/**
* This is a typical simple rule for testing that is easy for most basic testing
* @param ruleId
* @param enabled Enables the rule on creation or not. Defaulted to false to enable it on import
*/
export const getSimpleRule = (ruleId = 'rule-1'): CreateRulesSchema => ({
export const getSimpleRule = (ruleId = 'rule-1', enabled = false): CreateRulesSchema => ({
name: 'Simple Rule Query',
description: 'Simple Rule Query',
enabled,
risk_score: 1,
rule_id: ruleId,
severity: 'high',
@@ -360,12 +362,9 @@ export const deleteSignalsIndex = async (
* for testing uploads.
* @param ruleIds Array of strings of rule_ids
*/
export const getSimpleRuleAsNdjson = (ruleIds: string[], enabled?: boolean): Buffer => {
export const getSimpleRuleAsNdjson = (ruleIds: string[], enabled = false): Buffer => {
const stringOfRules = ruleIds.map((ruleId) => {
const simpleRule = getSimpleRule(ruleId);
if (enabled != null) {
return JSON.stringify({ enabled, ...simpleRule });
}
const simpleRule = getSimpleRule(ruleId, enabled);
return JSON.stringify(simpleRule);
});
return Buffer.from(stringOfRules.join('\n'));