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

[7.x] [Security Solutions][Detection Engine] Adds a warning banner when the alerts data has not been migrated yet. (#90258) #91602

Merged
merged 1 commit into from
Feb 17, 2021

Conversation

FrankHassanabad
Copy link
Contributor

Backports the following commits to 7.x:

… alerts data has not been migrated yet. (elastic#90258)

## Summary

Adds a warning banner for when the alerting/signals data has not been migrated to the new structure. Although we are planning on supporting some backwards compatibility where the rules don't completely blow up, this support of backwards compatibility is going to be best effort and not have explicit tests and checks against backwards compatibility. Hence the reason we need to alert any users of the system when we can that they should have an administrator visit the detections page to start a migration.

From previous reasons why we don't migrate on startup of Kibana is that there are multiple instances running and it might be a worse situation so we migrate on page visit by an administrator to reduce chances of issues. In the future we might revisit this decision but for now this is what we have moved forward with.

If the user does not have sufficient privileges such as t1 analyst to see if they have should upgrade, no message is shown to those users.

This PR adds the following banner which is non-dismissible to:
* Main detections page
* Manage rules page
* View/Edit rules page
<img width="2259" alt="Screen Shot 2021-02-03 at 4 16 00 PM" src="https://user-images.githubusercontent.com/1151048/106926989-eb2fb300-66ce-11eb-877c-1210357af108.png">

If other dismissible alerts are on the page then you will get a stacked effect until you dismiss those messages. Again, this message cannot be dismissed intentionally to let the user know that they should contact an administrator to update/upgrade the alerting/signal data:
<img width="1526" alt="Screen Shot 2021-02-03 at 5 41 57 PM" src="https://user-images.githubusercontent.com/1151048/106927465-6b561880-66cf-11eb-8c0f-dfdfa624c24b.png">

Other items of note:
* Added ability to remove the button from the callouts
* Consolidated in one area some types
* Removed one part of the callout that has branching logic we never activate. We can re-add that later if we do have a need for it
* e2e Cypress tests added to detect when the banner should be present
* Backfilled unit tests for enzyme for some of the callout code

Manual testing:
Bump this number in your dev env:
https://github.com/elastic/kibana/blob/master/x-pack/plugins/security_solution/server/lib/detection_engine/routes/index/get_signals_template.ts#L11

Give yourself these permissions (or use one of the scripts for creating these roles):
<img width="1243" alt="Screen Shot 2021-02-05 at 1 49 02 PM" src="https://user-images.githubusercontent.com/1151048/107087773-30301400-67b9-11eb-9ac9-0a67fafd8231.png">

Visit the page.

### Checklist

Delete any items that are not applicable to this PR.

- [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md)
- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
- [ ] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [ ] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))
- [ ] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))
- [x] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers)
@kibanamachine
Copy link
Contributor

💛 Build succeeded, but was flaky


Test Failures

Kibana Pipeline / general / "before all" hook for "should contain notes".Timeline notes tab "before all" hook for "should contain notes"

Link to Jenkins

Stack Trace

Failed Tests Reporter:
  - Test has not failed recently on tracked branches

AssertionError: Timed out retrying after 60000ms: Expected to find element: `[data-test-subj="title-1f6092a0-70e0-11eb-8e36-dbbba9c4a1b1"]`, but never found it.

Because this error occurred during a `before all` hook we are skipping the remaining tests in the current suite: `Timeline notes tab`

Although you have test retries enabled, we do not retry tests when `before all` or `after all` hooks fail
    at Object.openTimelineById (http://localhost:6111/__cypress/tests?p=cypress/integration/timelines/notes_tab.spec.ts:16007:15)
    at Context.eval (http://localhost:6111/__cypress/tests?p=cypress/integration/timelines/notes_tab.spec.ts:15041:24)

Kibana Pipeline / general / "after all" hook for "should contain notes".Timeline notes tab "after all" hook for "should contain notes"

Link to Jenkins

Stack Trace

Failed Tests Reporter:
  - Test has not failed recently on tracked branches

CypressError: `cy.filter()` failed because it requires a DOM element.

The subject received was:

  > `undefined`

The previous command that ran was:

  > `cy.get()`

All 2 subject validations failed on this subject.

Because this error occurred during a `after all` hook we are skipping the remaining tests in the current suite: `Timeline notes tab`

Although you have test retries enabled, we do not retry tests when `before all` or `after all` hooks fail
    at ensureElement (http://elastic:changeme@localhost:6111/__cypress/runner/cypress_runner.js:161322:24)
    at validateType (http://elastic:changeme@localhost:6111/__cypress/runner/cypress_runner.js:161159:16)
    at Object.ensureSubjectByType (http://elastic:changeme@localhost:6111/__cypress/runner/cypress_runner.js:161195:9)
    at pushSubjectAndValidate (http://elastic:changeme@localhost:6111/__cypress/runner/cypress_runner.js:169888:15)
    at Context.<anonymous> (http://elastic:changeme@localhost:6111/__cypress/runner/cypress_runner.js:170225:18)
From Your Spec Code:
    at Object.closeTimeline (http://localhost:6111/__cypress/tests?p=cypress/integration/timelines/notes_tab.spec.ts:15960:43)
    at Context.eval (http://localhost:6111/__cypress/tests?p=cypress/integration/timelines/notes_tab.spec.ts:15051:20)

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 2190 2192 +2

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
securitySolution 7.6MB 7.6MB +3.5KB

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

@FrankHassanabad FrankHassanabad merged commit 7bf9d63 into elastic:7.x Feb 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants