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

dhurley14 · 2020-09-17T14:36:12Z

Summary

io-ts validation on the rule import file object was creating performance issues. We replace this validation with kbn schema.any() since the validation on the rules occurs later in the route handler.

Checklist

Delete any items that are not applicable to this PR.

Unit or functional tests were updated or added to match the most common scenarios

For maintainers

This was checked for breaking API changes and was labeled appropriately

elasticmachine · 2020-09-17T14:47:45Z

Pinging @elastic/siem (Team:SIEM)

FrankHassanabad · 2020-09-17T15:10:45Z

x-pack/test/detection_engine_api_integration/basic/tests/import_rules.ts

+
+      // uncomment the below test once we speed up the alerts client find api
+      // in another PR.
+      // it('should be able to import 10000 rules', async () => {


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.

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

FrankHassanabad

LGTM, thanks for the work to fix our customers that are complaining about the slowness and this smaller PR here.

…quest, adds new functional tests and unit tests to make sure we can reach and don't go past bounds. These tests would have helped uncover performance issues io-ts gave us with validating the import rules file object

…param defaulted to false

kibanamachine · 2020-09-17T19:39:45Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: dd29bed

Build metrics

✅ unchanged

History

💔 Build #75227 failed 8f81a62d9b2a9952d457cd1ccb9494fe2bd67435

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

…te (elastic#77770) * utlize schema.any() for validation on file in body of import rules request, adds new functional tests and unit tests to make sure we can reach and don't go past bounds. These tests would have helped uncover performance issues io-ts gave us with validating the import rules file object * fix type check failure * updates getSimpleRule and getSimpleRuleAsNdjson to accept an enabled param defaulted to false * updates comments in e2e tests for import rules route * fix tests after adding enabled boolean in test utils

…rt route (#77770) (#77821) * utlize schema.any() for validation on file in body of import rules request, adds new functional tests and unit tests to make sure we can reach and don't go past bounds. These tests would have helped uncover performance issues io-ts gave us with validating the import rules file object * fix type check failure * updates getSimpleRule and getSimpleRuleAsNdjson to accept an enabled param defaulted to false * updates comments in e2e tests for import rules route * fix tests after adding enabled boolean in test utils

…rt route (#77770) (#77822) * utlize schema.any() for validation on file in body of import rules request, adds new functional tests and unit tests to make sure we can reach and don't go past bounds. These tests would have helped uncover performance issues io-ts gave us with validating the import rules file object * fix type check failure * updates getSimpleRule and getSimpleRuleAsNdjson to accept an enabled param defaulted to false * updates comments in e2e tests for import rules route * fix tests after adding enabled boolean in test utils

* master: (92 commits) [ILM] Data tiers for 7.10 (elastic#76126) [ML] Transforms: Fixes styling of preview grid pagination in summary step (elastic#77789) [Drilldowns] Beta badge support. Mark URL Drilldown as Beta (elastic#75654) Re-enable session lifespan, idle timeout api integration tests and use unique names for the security test reports. (elastic#77746) [Alerting] renames code in alerting RBAC exemption to make it easier to maintain (elastic#77598) [Alerting & Actions] Overwrite SOs when updating instead of partially updating (elastic#73688) fixed react warning in Suspense in alert flyout (elastic#77777) [APM] Track usage of Gold+ features (elastic#77630) Visualize: Bad request when working with histogram aggregation (elastic#77684) remove legacy ES plugin (elastic#77703) [Lens] change name of custom query to filters (elastic#77725) skip flaky suite (elastic#76239) remove visual aspects of baseline job (elastic#77815) skip flaky suite (elastic#77835) Fixes typo in data recognizer text (elastic#77691) management/update trusted_apps jest snapshot [build] Use Elastic hosted UBI minimal base image (elastic#77776) [APM] Add transaction error rate alert (elastic#76933) [Security Solution] [Detections] Remove file validation on import route (elastic#77770) [Enterprise Search][tech debt] Add Kea logic paths for easier debugging/defaults (elastic#77698) ...

elasticmachine · 2021-09-23T14:25:03Z

Pinging @elastic/security-solution (Team: SecuritySolution)

dhurley14 mentioned this pull request Sep 17, 2020

[Security Solution][Detections] Performance enhancement for readRules function #76398

Closed

2 tasks

dhurley14 self-assigned this Sep 17, 2020

dhurley14 added release_note:fix review v7.10.0 v7.9.2 v8.0.0 Team:SIEM Feature:Detection Rules Anything related to Security Solution's Detection Rules labels Sep 17, 2020

dhurley14 marked this pull request as ready for review September 17, 2020 14:47

dhurley14 requested review from a team as code owners September 17, 2020 14:47

FrankHassanabad reviewed Sep 17, 2020

View reviewed changes

FrankHassanabad approved these changes Sep 17, 2020

View reviewed changes

dhurley14 added 5 commits September 17, 2020 11:27

fix type check failure

e267e88

updates getSimpleRule and getSimpleRuleAsNdjson to accept an enabled …

fe888cf

…param defaulted to false

updates comments in e2e tests for import rules route

acfb15d

fix tests after adding enabled boolean in test utils

dd29bed

dhurley14 force-pushed the remove-validation-import-route branch from 8f81a62 to dd29bed Compare September 17, 2020 18:03

dhurley14 merged commit becb137 into elastic:master Sep 17, 2020

dhurley14 deleted the remove-validation-import-route branch September 17, 2020 20:27

This was referenced Sep 17, 2020

[7.x] [Security Solution] [Detections] Remove file validation on import route (#77770) #77821

Merged

[7.9] [Security Solution] [Detections] Remove file validation on import route (#77770) #77822

Merged

MindyRS added the Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. label Sep 23, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

dhurley14 commented Sep 17, 2020

elasticmachine commented Sep 17, 2020

FrankHassanabad Sep 17, 2020

dhurley14 Sep 17, 2020 •

edited

Loading

FrankHassanabad left a comment

kibanamachine commented Sep 17, 2020

elasticmachine commented Sep 23, 2021

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

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

Conversation

dhurley14 commented Sep 17, 2020

Summary

Checklist

For maintainers

elasticmachine commented Sep 17, 2020

FrankHassanabad Sep 17, 2020

Choose a reason for hiding this comment

dhurley14 Sep 17, 2020 • edited Loading

Choose a reason for hiding this comment

FrankHassanabad left a comment

Choose a reason for hiding this comment

kibanamachine commented Sep 17, 2020

💚 Build Succeeded

Build metrics

History

elasticmachine commented Sep 23, 2021

dhurley14 Sep 17, 2020 •

edited

Loading