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] Fix related_integrations Cypress test flake #173393

Merged
merged 6 commits into from
Dec 19, 2023

Conversation

jpdjere
Copy link
Contributor

@jpdjere jpdjere commented Dec 14, 2023

Summary

  • Replaces the installation of all available prebuilt rule for the rule that was just created. This stops flake in which a race condition would case the full Fleet package installation to be installed, and then us purposely installing all rules.

Flaky test runner

https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4574

For maintainers

@jpdjere jpdjere added 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 labels Dec 14, 2023
@jpdjere jpdjere self-assigned this Dec 14, 2023
@jpdjere jpdjere marked this pull request as ready for review December 14, 2023 15:10
@jpdjere jpdjere requested a review from a team as a code owner December 14, 2023 15:10
@jpdjere jpdjere requested a review from banderror December 14, 2023 15:10
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

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

@banderror
Copy link
Contributor

@elasticmachine merge upstream

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.

@jpdjere Thank you for quickly picking up this issue. I left a comment below.

I think I'd be ok with merging this PR despite the comment, because:

  • The code after the change doesn't look more flaky and more explicitly reflects our intentions.
  • The Flaky Test Runner is happy. The only failure was because an agent was lost.


const INSTALLED_PREBUILT_RULES_RESPONSE_ALIAS = 'prebuiltRules';

function addAndInstallPrebuiltRules(rules: Array<typeof SAMPLE_PREBUILT_RULE>): void {
installPrebuiltRuleAssets(rules);
installAllPrebuiltRulesRequest().as(INSTALLED_PREBUILT_RULES_RESPONSE_ALIAS);
installSpecificPrebuiltRulesRequest(rules).as(INSTALLED_PREBUILT_RULES_RESPONSE_ALIAS);
Copy link
Contributor

Choose a reason for hiding this comment

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

@jpdjere Not sure I understand how this change is supposed to reduce flakiness.

The installPrebuiltRuleAssets function calls preventPrebuiltRulesPackageInstallation under the hood, which means installAllPrebuiltRulesRequest should be installing only the test rules assets specified in this test, and not all real prebuilt rules from the package.

The "before" and "after" code look identical to me in terms of the end result.

On the other hand, I see this in the PR description: "This stops flake in which a race condition would case the full Fleet package installation to be installed, and then us purposely installing all rules." Can you elaborate on this?

@banderror
Copy link
Contributor

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

cc @jpdjere

@banderror banderror merged commit d0e5c29 into elastic:main Dec 19, 2023
37 checks passed
@kibanamachine
Copy link
Contributor

💔 All backports failed

Status Branch Result
8.12 Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 173393

Questions ?

Please refer to the Backport tool documentation

@banderror
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.12

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

Questions ?

Please refer to the Backport tool documentation

banderror pushed a commit to banderror/kibana that referenced this pull request Dec 20, 2023
…tic#173393)

## Summary

- Replaces the installation of all available prebuilt rule for the rule
that was just created. This stops flake in which a race condition would
case the full Fleet package installation to be installed, and then us
purposely installing **all** rules.

## Flaky test runner

https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4574

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

---------

Co-authored-by: Kibana Machine <[email protected]>
(cherry picked from commit d0e5c29)

# Conflicts:
#	x-pack/test/security_solution_cypress/cypress/e2e/detection_response/rule_management/related_integrations/related_integrations.cy.ts
banderror added a commit that referenced this pull request Dec 20, 2023
#173393) (#173680)

# Backport

This will backport the following commits from `main` to `8.12`:
- [[Security Solution] Fix related_integrations Cypress test flake
(#173393)](#173393)

<!--- Backport version: 8.9.8 -->

### 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-12-19T13:00:17Z","message":"[Security
Solution] Fix related_integrations Cypress test flake (#173393)\n\n##
Summary\r\n\r\n- Replaces the installation of all available prebuilt
rule for the rule\r\nthat was just created. This stops flake in which a
race condition would\r\ncase the full Fleet package installation to be
installed, and then us\r\npurposely installing **all** rules.\r\n\r\n##
Flaky test
runner\r\n\r\nhttps://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4574\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#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by:
Kibana Machine
<[email protected]>","sha":"d0e5c290789b7a5bb40a4099ede169dafc6f5ad3","branchLabelMapping":{"^v8.13.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:Detections
and Resp","Team: SecuritySolution","Team:Detection Rule
Management","v8.12.0","v8.13.0"],"number":173393,"url":"https://github.com/elastic/kibana/pull/173393","mergeCommit":{"message":"[Security
Solution] Fix related_integrations Cypress test flake (#173393)\n\n##
Summary\r\n\r\n- Replaces the installation of all available prebuilt
rule for the rule\r\nthat was just created. This stops flake in which a
race condition would\r\ncase the full Fleet package installation to be
installed, and then us\r\npurposely installing **all** rules.\r\n\r\n##
Flaky test
runner\r\n\r\nhttps://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4574\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#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by:
Kibana Machine
<[email protected]>","sha":"d0e5c290789b7a5bb40a4099ede169dafc6f5ad3"}},"sourceBranch":"main","suggestedTargetBranches":["8.12"],"targetPullRequestStates":[{"branch":"8.12","label":"v8.12.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.13.0","labelRegex":"^v8.13.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/173393","number":173393,"mergeCommit":{"message":"[Security
Solution] Fix related_integrations Cypress test flake (#173393)\n\n##
Summary\r\n\r\n- Replaces the installation of all available prebuilt
rule for the rule\r\nthat was just created. This stops flake in which a
race condition would\r\ncase the full Fleet package installation to be
installed, and then us\r\npurposely installing **all** rules.\r\n\r\n##
Flaky test
runner\r\n\r\nhttps://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4574\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#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by:
Kibana Machine
<[email protected]>","sha":"d0e5c290789b7a5bb40a4099ede169dafc6f5ad3"}}]}]
BACKPORT-->

Co-authored-by: Juan Pablo Djeredjian <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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. v8.12.0 v8.13.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants