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.

Sign up for GitHub

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

Jump to bottom

[Security Solution] Create test plan for prebuilt rule installation and upgrade workflows #160685

Merged
merged 16 commits into from
Jul 5, 2023
Merged

[Security Solution] Create test plan for prebuilt rule installation and upgrade workflows #160685

merged 16 commits into from
Jul 5, 2023

Conversation

jpdjere
Copy link
Contributor

@jpdjere jpdjere commented Jun 27, 2023 •
edited by kibanamachine
Loading

Summary

Combines the two following test plans into one:

Adds a template for writing new test plans.

@jpdjere jpdjere requested a review from vgomez-el June 27, 2023 22:36
@jpdjere jpdjere self-assigned this Jun 27, 2023
@jpdjere jpdjere added test-plan release_note:skip Skip the PR/issue when compiling release notes Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Detection Rule Management Security Detection Rule Management Team v8.9.0 Feature:Prebuilt Detection Rules Security Solution Prebuilt Detection Rules area labels Jun 28, 2023
@jpdjere jpdjere marked this pull request as ready for review June 28, 2023 12:01
@jpdjere jpdjere requested a review from a team as a code owner June 28, 2023 12:01
@jpdjere jpdjere requested a review from spong June 28, 2023 12:01
@elasticmachine
Copy link
Contributor

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

@banderror
Copy link
Contributor

@elasticmachine merge upstream

@banderror banderror mentioned this pull request Jul 3, 2023
Merged
1 task
jpdjere
jpdjere commented Jul 4, 2023
View reviewed changes
Copy link
Contributor Author

@jpdjere jpdjere left a comment

Choose a reason for hiding this comment

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

Reviewed and decided on changes with @banderror . LGTM

@banderror
Copy link
Contributor

@elasticmachine merge upstream

banderror
banderror approved these changes Jul 4, 2023
View reviewed changes
Copy link
Contributor

@banderror banderror left a comment

Choose a reason for hiding this comment

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

I pushed the final fixes to the plan and cleaned up the diff.

Every scenario was discussed with @jpdjere in detail, and we're going to work on adding more tests in follow-up PRs this and next week after merging the initial test coverage in #155241.

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Unknown metric groups

ESLint disabled line counts

id before after diff
enterpriseSearch 14 16 +2
securitySolution 410 414 +4
total +6

Total ESLint disabled count

id before after diff
enterpriseSearch 15 17 +2
securitySolution 489 493 +4
total +6

History

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

cc @jpdjere @banderror

@banderror banderror requested a review from a team as a code owner July 4, 2023 18:42
vgomez-el
vgomez-el reviewed Jul 5, 2023
View reviewed changes
...ty_solution/cypress/test_plans/detection_response/prebuilt_rules/installation_and_upgrade.md
| A | B |
| 8.7 | 8.9.0 |
| 7.17.x | 8.9.0 |
```
Copy link

Choose a reason for hiding this comment

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

B is always 8.9, why don't just leave it directly as 8.9 to improve readability?

Copy link
Contributor

Choose a reason for hiding this comment

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

@vgomez-el Because 8.9.0 is not the only target ("B") version that this scenario should work with. When 8.9.0 is released, the scenario will apply to 8.10.0, etc.

vgomez-el
vgomez-el reviewed Jul 5, 2023
View reviewed changes
...ty_solution/cypress/test_plans/detection_response/prebuilt_rules/installation_and_upgrade.md
### Non-functional requirements

- Notifications, rule installation and rule upgrade workflows should work:
- regardless of the package type: with historical rule versions or without;
Copy link

@vgomez-el vgomez-el Jul 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

What does Historical versions mean? Maybe previous versions? (i.e: 8.2, 7.17?)

Copy link
Contributor

Choose a reason for hiding this comment

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

@vgomez-el Great question! This is an architectural aspect. We ship historical (1 latest + N previous) versions of each prebuilt rule in the prebuilt rules package. So it's not about the stack version, and not about the version of the prebuilt rules package itself.

More info about this can be found here:

@vgomez-el
Copy link

Good job expanding the scenarios checking the API calls too and adding a more technical approach rather than just focusing on the functional part. This test plan goes beyond what a regular functional Test Plan would be, but it is fine for now. I just have a few comments in general:

  • This test plan covers scenarios that are going to be automated using, cypress for e2e tests, Jest for integration tests and Unit tests but it is under x-pack/plugins/security_solution/cypress/. IMO Test plans should be technology agnostic, and today we are using cypress, but in the future we can potentially use any other technology, that is why it does not make too much sense to have this test plans inside a specific framework technology folder.
  • There are some scenarios like this one for prebuilt rules installation: or this other one for prebuilt rules upgrade, that properly specifies the values of Y, but there are some other scenarios that just use: N rules, N and N-1 package versions, X prebuilt rules, X Installed rules, Z, etc
    Those abstract values can be confusing for other people outside the team who want to read, execute or contribute to the test plan because they might not know what values to use for testing.
    We should specify those values with examples or equivalence tables to clarify which values can those variables have, considering boundary values and special or critical versions, i.e. Most of our users are currently on 7.17.x versions, so IMO it is going to be useful making a distinction between most used versions or other versions with big changes. A good example of this is the Kibana upgrade scenario

@banderror
Copy link
Contributor

This test plan covers scenarios that are going to be automated using, cypress for e2e tests, Jest for integration tests and Unit tests but it is under x-pack/plugins/security_solution/cypress/. IMO Test plans should be technology agnostic, and today we are using cypress, but in the future we can potentially use any other technology, that is why it does not make too much sense to have this test plans inside a specific framework technology folder.

@vgomez-el Good point 👍 I can move the test_plans folder outside of cypress. Any specific suggestions? I'm thinking about security_solution/docs/test_plans. I can address this in a separate PR.

There are some scenarios like this one for prebuilt rules installation: or this other one for prebuilt rules upgrade, that properly specifies the values of Y, but there are some other scenarios that just use: N rules, N and N-1 package versions, X prebuilt rules, X Installed rules, Z, etc
Those abstract values can be confusing for other people outside the team who want to read, execute or contribute to the test plan because they might not know what values to use for testing.
We should specify those values with examples or equivalence tables to clarify which values can those variables have

We can do that. Would you be willing to open a PR with this kind of improvement? Both @jpdjere and I are overloaded with urgent work to do.

considering boundary values and special or critical versions, i.e. Most of our users are currently on 7.17.x versions, so IMO it is going to be useful making a distinction between most used versions or other versions with big changes. A good example of this is the Kibana upgrade scenario

This feature doesn't technically depend on any specific Kibana version, that's why we're not mentioning it in any scenario except the upgrade one. I think in general scenarios should assume the latest version of Kibana, since they are kept in the main branch. We might want to specify concrete Kibana versions in upgrade scenarios or maybe in some edge cases when we want to cover a specific well-known issue in some version.

@banderror banderror merged commit e379f0a into elastic:main Jul 5, 2023
@kibanamachine kibanamachine mentioned this pull request Jul 5, 2023
Merged
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jul 5, 2023
@jpdjere
[Security Solution] Create test plan for prebuilt rule installation a…
3a1308d 
…nd upgrade workflows (elastic#160685)

## Summary

Combines the two following test plans into one:

- [Test plan for the legacy workflows of installing/upgrading prebuilt
rules](https://docs.google.com/document/d/1d_1DYnHlnCaPznWTjeCxhoaRUwxc2O_V0LToAPG0xLE/edit)
- this google doc is deprecated and will be replaced by the consolidated
test plan created in this PR
- [Test plan for the new workflows of installing/upgrading prebuilt
rules](https://docs.google.com/document/d/1cYvwtpzk0uLn5R88BlCX-fRIwR2n_NoqkSjIXkY9k34/edit)
- this google doc is deprecated and will be replaced by the consolidated
test plan created in this PR

Adds a template for writing new test plans.

---------

Co-authored-by: Georgii Gorbachev <[email protected]>
Co-authored-by: Kibana Machine <[email protected]>
(cherry picked from commit e379f0a)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
✅ 8.9

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Jul 5, 2023
@kibanamachine @jpdjere
[8.9] [Security Solution] Create test plan for prebuilt rule installa…
cd4bd06 
…tion and upgrade workflows (#160685) (#161259)

# Backport

This will backport the following commits from `main` to `8.9`:
- [[Security Solution] Create test plan for prebuilt rule installation
and upgrade workflows
(#160685)](#160685)

<!--- Backport version: 8.9.7 -->

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

<!--BACKPORT [{"author":{"name":"Juan Pablo
Djeredjian","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-07-05T13:50:20Z","message":"[Security
Solution] Create test plan for prebuilt rule installation and upgrade
workflows (#160685)\n\n## Summary\r\n\r\nCombines the two following test
plans into one:\r\n\r\n- [Test plan for the legacy workflows of
installing/upgrading
prebuilt\r\nrules](https://docs.google.com/document/d/1d_1DYnHlnCaPznWTjeCxhoaRUwxc2O_V0LToAPG0xLE/edit)\r\n-
this google doc is deprecated and will be replaced by the
consolidated\r\ntest plan created in this PR\r\n- [Test plan for the new
workflows of installing/upgrading
prebuilt\r\nrules](https://docs.google.com/document/d/1cYvwtpzk0uLn5R88BlCX-fRIwR2n_NoqkSjIXkY9k34/edit)\r\n-
this google doc is deprecated and will be replaced by the
consolidated\r\ntest plan created in this PR\r\n\r\nAdds a template for
writing new test plans.\r\n\r\n---------\r\n\r\nCo-authored-by: Georgii
Gorbachev <[email protected]>\r\nCo-authored-by: Kibana
Machine
<[email protected]>","sha":"e379f0a97d66b53e130f1046811225b0fa91678a","branchLabelMapping":{"^v8.10.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","test-plan","Team:Detections
and Resp","Team: SecuritySolution","Team:Detection Rule
Management","Feature:Prebuilt Detection
Rules","v8.9.0","v8.10.0"],"number":160685,"url":"https://github.com/elastic/kibana/pull/160685","mergeCommit":{"message":"[Security
Solution] Create test plan for prebuilt rule installation and upgrade
workflows (#160685)\n\n## Summary\r\n\r\nCombines the two following test
plans into one:\r\n\r\n- [Test plan for the legacy workflows of
installing/upgrading
prebuilt\r\nrules](https://docs.google.com/document/d/1d_1DYnHlnCaPznWTjeCxhoaRUwxc2O_V0LToAPG0xLE/edit)\r\n-
this google doc is deprecated and will be replaced by the
consolidated\r\ntest plan created in this PR\r\n- [Test plan for the new
workflows of installing/upgrading
prebuilt\r\nrules](https://docs.google.com/document/d/1cYvwtpzk0uLn5R88BlCX-fRIwR2n_NoqkSjIXkY9k34/edit)\r\n-
this google doc is deprecated and will be replaced by the
consolidated\r\ntest plan created in this PR\r\n\r\nAdds a template for
writing new test plans.\r\n\r\n---------\r\n\r\nCo-authored-by: Georgii
Gorbachev <[email protected]>\r\nCo-authored-by: Kibana
Machine
<[email protected]>","sha":"e379f0a97d66b53e130f1046811225b0fa91678a"}},"sourceBranch":"main","suggestedTargetBranches":["8.9"],"targetPullRequestStates":[{"branch":"8.9","label":"v8.9.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.10.0","labelRegex":"^v8.10.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/160685","number":160685,"mergeCommit":{"message":"[Security
Solution] Create test plan for prebuilt rule installation and upgrade
workflows (#160685)\n\n## Summary\r\n\r\nCombines the two following test
plans into one:\r\n\r\n- [Test plan for the legacy workflows of
installing/upgrading
prebuilt\r\nrules](https://docs.google.com/document/d/1d_1DYnHlnCaPznWTjeCxhoaRUwxc2O_V0LToAPG0xLE/edit)\r\n-
this google doc is deprecated and will be replaced by the
consolidated\r\ntest plan created in this PR\r\n- [Test plan for the new
workflows of installing/upgrading
prebuilt\r\nrules](https://docs.google.com/document/d/1cYvwtpzk0uLn5R88BlCX-fRIwR2n_NoqkSjIXkY9k34/edit)\r\n-
this google doc is deprecated and will be replaced by the
consolidated\r\ntest plan created in this PR\r\n\r\nAdds a template for
writing new test plans.\r\n\r\n---------\r\n\r\nCo-authored-by: Georgii
Gorbachev <[email protected]>\r\nCo-authored-by: Kibana
Machine
<[email protected]>","sha":"e379f0a97d66b53e130f1046811225b0fa91678a"}}]}]
BACKPORT-->

Co-authored-by: Juan Pablo Djeredjian <[email protected]>
This was referenced Jul 6, 2023
Closed
Merged
banderror added a commit that referenced this pull request Jul 11, 2023
@banderror
[Security Solution] Move test plans from /cypress to /docs (#161517)
15a86c3 
**Related to:** elastic/security-team#6867
(internal)

## Summary

As requested in
#160685 (comment).

In test plans, we mention how scenarios are going to be automated --
whether a given scenario will be automated using Cypress for e2e tests,
Jest for integration tests and unit tests, etc. But currently, test
plans are under `x-pack/plugins/security_solution/cypress/`.

Since test plans are in fact technology agnostic, it does not make much
sense to keep them inside a specific framework technology folder. That's
why we're moving them to a generic
`x-pack/plugins/security_solution/docs/testing` folder.
banderror added a commit to banderror/kibana that referenced this pull request Jul 12, 2023
@banderror
[Security Solution] Move test plans from /cypress to /docs (elastic#1…
44899f2 
…61517)

**Related to:** elastic/security-team#6867
(internal)

## Summary

As requested in
elastic#160685 (comment).

In test plans, we mention how scenarios are going to be automated --
whether a given scenario will be automated using Cypress for e2e tests,
Jest for integration tests and unit tests, etc. But currently, test
plans are under `x-pack/plugins/security_solution/cypress/`.

Since test plans are in fact technology agnostic, it does not make much
sense to keep them inside a specific framework technology folder. That's
why we're moving them to a generic
`x-pack/plugins/security_solution/docs/testing` folder.

(cherry picked from commit 15a86c3)

# Conflicts:
#	.github/CODEOWNERS
banderror added a commit that referenced this pull request Jul 13, 2023
@banderror
[8.9] [Security Solution] Move test plans from /cypress to /docs (#16…
ee95a11 
…1517) (#161739)

# Backport

This will backport the following commits from `main` to `8.9`:
- [[Security Solution] Move test plans from /cypress to /docs
(#161517)](#161517)

<!--- Backport version: 8.9.7 -->

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

<!--BACKPORT [{"author":{"name":"Georgii
Gorbachev","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-07-11T09:27:00Z","message":"[Security
Solution] Move test plans from /cypress to /docs (#161517)\n\n**Related
to:**
https://github.com/elastic/security-team/issues/6867\r\n(internal)\r\n\r\n##
Summary\r\n\r\nAs requested
in\r\nhttps://github.com//pull/160685#issuecomment-1621635262.\r\n\r\nIn
test plans, we mention how scenarios are going to be automated
--\r\nwhether a given scenario will be automated using Cypress for e2e
tests,\r\nJest for integration tests and unit tests, etc. But currently,
test\r\nplans are under
`x-pack/plugins/security_solution/cypress/`.\r\n\r\nSince test plans are
in fact technology agnostic, it does not make much\r\nsense to keep them
inside a specific framework technology folder. That's\r\nwhy we're
moving them to a
generic\r\n`x-pack/plugins/security_solution/docs/testing`
folder.","sha":"15a86c355a261ce11cad82f3b3c47dfeca814d1f","branchLabelMapping":{"^v8.10.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","test-plan","Team:Detections
and Resp","Team: SecuritySolution","Team:Detection Rule
Management","v8.9.0","v8.10.0"],"number":161517,"url":"https://github.com/elastic/kibana/pull/161517","mergeCommit":{"message":"[Security
Solution] Move test plans from /cypress to /docs (#161517)\n\n**Related
to:**
https://github.com/elastic/security-team/issues/6867\r\n(internal)\r\n\r\n##
Summary\r\n\r\nAs requested
in\r\nhttps://github.com//pull/160685#issuecomment-1621635262.\r\n\r\nIn
test plans, we mention how scenarios are going to be automated
--\r\nwhether a given scenario will be automated using Cypress for e2e
tests,\r\nJest for integration tests and unit tests, etc. But currently,
test\r\nplans are under
`x-pack/plugins/security_solution/cypress/`.\r\n\r\nSince test plans are
in fact technology agnostic, it does not make much\r\nsense to keep them
inside a specific framework technology folder. That's\r\nwhy we're
moving them to a
generic\r\n`x-pack/plugins/security_solution/docs/testing`
folder.","sha":"15a86c355a261ce11cad82f3b3c47dfeca814d1f"}},"sourceBranch":"main","suggestedTargetBranches":["8.9"],"targetPullRequestStates":[{"branch":"8.9","label":"v8.9.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.10.0","labelRegex":"^v8.10.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/161517","number":161517,"mergeCommit":{"message":"[Security
Solution] Move test plans from /cypress to /docs (#161517)\n\n**Related
to:**
https://github.com/elastic/security-team/issues/6867\r\n(internal)\r\n\r\n##
Summary\r\n\r\nAs requested
in\r\nhttps://github.com//pull/160685#issuecomment-1621635262.\r\n\r\nIn
test plans, we mention how scenarios are going to be automated
--\r\nwhether a given scenario will be automated using Cypress for e2e
tests,\r\nJest for integration tests and unit tests, etc. But currently,
test\r\nplans are under
`x-pack/plugins/security_solution/cypress/`.\r\n\r\nSince test plans are
in fact technology agnostic, it does not make much\r\nsense to keep them
inside a specific framework technology folder. That's\r\nwhy we're
moving them to a
generic\r\n`x-pack/plugins/security_solution/docs/testing`
folder.","sha":"15a86c355a261ce11cad82f3b3c47dfeca814d1f"}}]}]
BACKPORT-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vgomez-el vgomez-el vgomez-el left review comments

@banderror banderror banderror approved these changes

Assignees

@jpdjere jpdjere

@banderror banderror

Labels
Feature:Prebuilt Detection Rules Security Solution Prebuilt Detection Rules area release_note:skip Skip the PR/issue when compiling release notes Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. test-plan v8.9.0 v8.10.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@jpdjere @elasticmachine @banderror @kibana-ci @vgomez-el @kibanamachine