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 ] Fix flake in bundled_prebuilt_rules_package/prerelease_packages.ts API Integration test #169780

Merged
merged 10 commits into from
Oct 30, 2023
Merged

[Security Solution ] Fix flake in bundled_prebuilt_rules_package/prerelease_packages.ts API Integration test #169780

merged 10 commits into from
Oct 30, 2023

Conversation

jpdjere
Copy link
Contributor

@jpdjere jpdjere commented Oct 25, 2023 •
edited by kibanamachine
Loading

Fixes: #162192

Summary

Removes flake by explicitly calling the Fleet endpoint to install the latest package, and doing assertions before actually installing the rules.

Previously we were calling directly installPrebuiltRules without having explicitly installed the package before. The old installation endpoint would check that the package was installed before proceeding, but the new install method doesn't. So the explicit installation is required.

Flaky test runs

@jpdjere jpdjere force-pushed the fix-prerelease-packages-flake branch from 40d884d to 335bffa Compare October 25, 2023 11:00
@jpdjere jpdjere self-assigned this Oct 26, 2023
@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 Feature:Prebuilt Detection Rules Security Solution Prebuilt Detection Rules area v8.12.0 v8.11.1 labels Oct 26, 2023
@jpdjere jpdjere marked this pull request as ready for review October 26, 2023 09:39
@jpdjere jpdjere requested a review from a team as a code owner October 26, 2023 09:39
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

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

@jpdjere jpdjere requested a review from maximpn October 26, 2023 11:10
maximpn

This comment was marked as duplicate.

Sign in to view

@jpdjere
Copy link
Contributor Author

jpdjere commented Oct 27, 2023

@maximpn

Refactored this PR to include one additional step to explicitly call the Fleet endpoint to install the latest package, and doing assertions before actually installing the rules.

Previously we were calling directly installPrebuiltRules without having explicitly installed the package before. The old installation endpoint would check that the package was installed before proceeding, but the new install method doesn't. So the explicit installation is required.

I'll run the flaky test runner, but please re-review. Thanks!

@jpdjere jpdjere requested a review from maximpn October 30, 2023 09:40
maximpn
maximpn approved these changes Oct 30, 2023
View reviewed changes
Copy link
Contributor

@maximpn maximpn left a comment

Choose a reason for hiding this comment

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

LGTM

@jpdjere could you add detailed description for installPrebuiltRulesPackageViaFleetAPI, installPrebuiltRules and installPrebuiltRulesPackageByVersion? So the difference is clear for any unprepared reader. It's easy to forget what the functions do exactly and I spend some time to remember. For example installPrebuiltRulesPackageViaFleetAPI and installPrebuiltRulesPackageByVersion have the same description Installs prebuilt rules package security_detection_engine by version. and it's so confusing.

@jpdjere jpdjere force-pushed the fix-prerelease-packages-flake branch from ca9f678 to 0e1a232 Compare October 30, 2023 11:30
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

✅ unchanged

History

  • 💚 Build #171586 succeeded 3cd3e26b622bf059e15b97d298b92a21dc87b9b0
  • 💔 Build #171552 failed afccc3c8a4f1a2615bcaf40057524346a5640645
  • 💚 Build #171126 succeeded 0f516ba99a7d7d50d2a32a74fe8d4f00835e6ba8
  • 💔 Build #170790 failed 24a32aef5b4c1811de52b1121927056007cd1bda
  • 💔 Build #170755 failed f33b94b1efd4c87582faba3785e6c521f116defe

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

cc @jpdjere

@jpdjere jpdjere merged commit d1b7b7f into elastic:main Oct 30, 2023
@kibanamachine kibanamachine mentioned this pull request Oct 30, 2023
Closed
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 30, 2023
@jpdjere
[Security Solution ] Fix flake in `bundled_prebuilt_rules_package/pre…
450314b 
…release_packages.ts` API Integration test (elastic#169780)

Fixes: elastic#162192

## Summary

Removes flake by explicitly calling the Fleet endpoint to install the
latest package, and doing assertions before actually installing the
rules.

Previously we were calling directly `installPrebuiltRules` without
having explicitly installed the package before. The old installation
endpoint would check that the package was installed before proceeding,
but the new install method doesn't. So the explicit installation is
required.

## Flaky test runs

-
~https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/3721~
-
~https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/3730
[CONTROL - NO CHANGES]~
-
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/3773
[After refactor]

(cherry picked from commit d1b7b7f)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
✅ 8.11

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

Questions ?

Please refer to the Backport tool documentation

@jpdjere jpdjere mentioned this pull request Oct 30, 2023
Closed
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 16, 2024
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add the label auto-backport or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 169780 locally

11 similar comments
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add the label auto-backport or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 169780 locally

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add the label auto-backport or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 169780 locally

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add the label auto-backport or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 169780 locally

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add the label auto-backport or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 169780 locally

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add the label auto-backport or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 169780 locally

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add the label auto-backport or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 169780 locally

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add the label auto-backport or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 169780 locally

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add the label auto-backport or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 169780 locally

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add the label auto-backport or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 169780 locally

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add the label auto-backport or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 169780 locally

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add the label auto-backport or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 169780 locally

@maximpn maximpn removed the v8.11.1 label Apr 15, 2024
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add the label auto-backport or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 169780 locally

@maximpn maximpn added backport:skip This commit does not require backporting and removed backport missing Added to PRs automatically when the are determined to be missing a backport. labels Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maximpn maximpn maximpn approved these changes

Assignees

@jpdjere jpdjere

Labels
backport:skip This commit does not require backporting 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. v8.12.0
Projects
None yet
Milestone
No milestone
5 participants
@jpdjere @elasticmachine @kibana-ci @kibanamachine @maximpn