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 Solutions] Add a setting to disable cloud rules for our CI system #105980

Closed
FrankHassanabad opened this issue Jul 16, 2021 · 2 comments · Fixed by #106189
Closed

[Security Solutions] Add a setting to disable cloud rules for our CI system #105980

FrankHassanabad opened this issue Jul 16, 2021 · 2 comments · Fixed by #106189
Assignees
Labels
discuss Team:Detections and Resp Security Detection Response Team

Comments

@FrankHassanabad
Copy link
Contributor

FrankHassanabad commented Jul 16, 2021

A few days ago we had an issue where a missing back-port of detection engine rules for 7.14. The backport went back to 7.15 and master but not 7.14. Cypress then began failing on everyone's new PR's. We fixed Cypress by doing a backport (#105790)

However, this started a conversation where it looks like either missing backports or changes to cloud rules can effect our CI system tests because we now always look for cloud rules within the backend route:
/api/detection_engine/rules/prepackaged

Whenever the cloud rule server contains more rules than the local file system, our Cypress tests will do an install and begin failing as we use counts of the local rules on the file system.

Options we discussed:

0) Change the Cypress tests to use ">=" in the tests

PROS:

CONS:

  • Requires developer discipline to always do some tests around prepackaged using this way
  • It weakens tests as some of the tests rely on using === for the tests and we can have tests passing which should fail.
  • One test not written like this causes all this work to become a problem again
  • Slightly more complex test reading and understanding

1) Change the Cypress tests to use counts from the prepackaged route

PROS:

  • It is the truth of how many rules are installed.
  • It allows us to get the real counts regardless of where they come from

CONS:

  • Requires developer discipline to always do some tests around prepackaged using this way
  • It weakens tests as the prepackaged counts aren't the "truth". The counts from the file system are. If you install less rules than you were counting on, tests would still pass.
  • cy.intercept is noisy in tests and sometimes has bugs. We increase our chances of introducing flake.
  • One test not written like this causes all this work to become a problem again

2) Change the prepackaed route to accept parameter to only install from disk

PROS:

  • Tests aren't weakened other than changing the route call slightly
  • You get a new REST endpoint feature for users

CONS:

  • Requires developer discipline to always use cy.intercept to add the flag
  • Changes our tests to not be the same as the end user's typical web experience so it's not exactly the same e2e
  • You're now testing only 1 of 2 possible route conditions of install all on disk vs. not on disk

3) Add a new kibana.yml setting to disable cloud rule checks and then use the setting within the cypress config file

PROS:

  • No developer discipline needed.
  • Tests aren't weakened and stay the same.
  • You could make this into a feature if you wanted for turning cloud rule checks off at some point in the future.

CONS:

  • Cypress cannot test cloud rules down the road if you wanted to (possibly). You might not want Cypress to test cloud rules this way or for it to test them at all though. You might only want e2e or some other test containers. So this might be a pro too.
  • Someone writes some code but it should be straight forward.
  • If we get the flags wrong or break something on refactoring, then users are going to begin losing cloud rule installs and we will not know unless we write backend e2e tests which can contact a cloud server for cloud rules and do a real e2e test of the rules. fwiw, we already have this problem today where we do not test the cloud server as far as I know.

So far we are leaning towards option 3:
3) Add a new kibana.yml setting to disable cloud rule checks and then use the setting within the cypress config file

@FrankHassanabad FrankHassanabad added the Team:Detections and Resp Security Detection Response Team label Jul 16, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detections-response (Team:Detections and Resp)

@MadameSheema
Copy link
Member

Agree with option 3 if needed we can think about how to expand the current tests to have more coverage regarding the other rules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Team:Detections and Resp Security Detection Response Team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants