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][Detection Engine] Fixes cypress errors by using latest signals mapping #84600

Merged
merged 4 commits into from
Dec 7, 2020

Conversation

FrankHassanabad
Copy link
Contributor

@FrankHassanabad FrankHassanabad commented Dec 1, 2020

Summary

Fixes issues within Cypress whenever developers would re-run tests they could become blocked as some of the tests cause a migration/roll over of the signals.

To reproduce the error off of master:

Start Cypress

cd ./kibana/x-pack/plugins/security_solution
yarn cypress:open-as-ci

Then run the export test twice. The first time it will run ok. The second time the test will refuse to run.

Screen Shot 2020-11-30 at 1 57 24 PM

The second time that you run the test you will get these errors from Cypress and will be blocked until you do workarounds such as logging into https://localhost:5620 and manually changing the indexes manually or restarting Cypress altogether.
Screen Shot 2020-11-30 at 6 05 07 PM

What is going on with the errors is that a migration is occurring since the existing signals mapping for the tests are not the newer migrated version and these are creating a new index named .siem-signals-default-0000002 like so:
Screen Shot 2020-11-30 at 1 57 12 PM

This index is not being cleaned up and when the next time we do an es_archive load we are marking two indexes as being writable

file: x-pack/test/security_solution_cypress/es_archives/alerts/mappings.json

".siem-signals-default": {
  "is_write_index": true
  }
},
"index": ".siem-signals-default-000001",

which leads to the stack trace and the Cypress errors on the front end:

MacBook-Pro.local] path: /.siem-signals-default-000001, params: {index=.siem-signals-default-000001}
           java.lang.IllegalStateException: alias [.siem-signals-default] has more than one write index [.siem-signals-default-000002,.siem-signals-default-000001]
               at org.elasticsearch.cluster.metadata.IndexAbstraction$Alias.computeAndValidateAliasProperties(IndexAbstraction.java:276) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]

This fixes that by updating the mapping and migration number. Each time we migrate/change the signals mapping we will have to perform a PR like this to update each location.

At the moment this is 5 different locations we have to update with the latest mappings.

Checklist

@FrankHassanabad FrankHassanabad changed the title Fixes dev testing experience [Security Solutions][Detection Engine] Fixes cypress errors by using latest signals mapping Dec 1, 2020
@FrankHassanabad FrankHassanabad self-assigned this Dec 1, 2020
@FrankHassanabad FrankHassanabad added Team:Detections and Resp Security Detection Response Team Feature:Detection Alerts Security Solution Detection Alerts Feature v8.0.0 v7.11.0 release_note:skip Skip the PR/issue when compiling release notes labels Dec 1, 2020
@FrankHassanabad FrankHassanabad marked this pull request as ready for review December 1, 2020 15:57
Copy link
Contributor

@rylnd rylnd left a comment

Choose a reason for hiding this comment

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

First: both the explanation and the steps are greatly appreciated.

I checked things out locally and verified both the "red" and "green" cases (🎄) as described in the summary 👍 .

I had one question/concern about all of the churn in these mappings files, but that's more of a noise-minimization problem on top of these already noisy files and not a blocker to merging this.

"properties": {
"@timestamp": {
"type": "date"
},
"agent": {
"properties": {
"ephemeral_id": {
"ignore_above": 1024,
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you know the cause behind all this churn? Our hope was that consistent use of es_archiver would minimize this sort of thing, but if its output is nondeterministic then that's worrisome.

Copy link
Contributor Author

@FrankHassanabad FrankHassanabad Dec 3, 2020

Choose a reason for hiding this comment

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

I copied and pasted this directly from the output of the mapping by doing a query rather than using es_archiver for this part. I don't think es_archiver sorts keys which is part of the problem vs. using something like dev tools to do an update. But overall we should make a better way of doing things as discussed during team meetings rather than doing large es archiver dumps of saved objects and signals due to migration issues and unsupported hot re-loading issues we are seeing.

@FrankHassanabad
Copy link
Contributor Author

@elasticmachine merge upstream

@FrankHassanabad
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Distributable file count

id before after diff
default 46929 47689 +760

History

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

@FrankHassanabad FrankHassanabad merged commit 1bc42b9 into elastic:master Dec 7, 2020
@FrankHassanabad FrankHassanabad deleted the fix-dev-errors branch December 7, 2020 22:51
FrankHassanabad added a commit to FrankHassanabad/kibana that referenced this pull request Dec 7, 2020
…latest signals mapping (elastic#84600)

## Summary

Fixes issues within Cypress whenever developers would re-run tests they could become blocked as some of the tests cause a migration/roll over of the signals.
 
To reproduce the error off of master:

Start Cypress 
```ts
cd ./kibana/x-pack/plugins/security_solution
yarn cypress:open-as-ci
```

Then run the export test twice. The first time it will run ok. The second time the test will refuse to run.

<img width="585" alt="Screen Shot 2020-11-30 at 1 57 24 PM" src="https://user-images.githubusercontent.com/1151048/100683706-2233c200-3336-11eb-949f-48f86e884d8b.png">

The second time that you run the test you will get these errors from Cypress and will be blocked until you do workarounds such as logging into `https://localhost:5620` and manually changing the indexes manually or restarting Cypress altogether.
<img width="1067" alt="Screen Shot 2020-11-30 at 6 05 07 PM" src="https://user-images.githubusercontent.com/1151048/100683945-9a9a8300-3336-11eb-9d49-187dee4fc1dc.png">

What is going on with the errors is that a migration is occurring since the existing signals mapping for the tests are not the newer migrated version and these are creating a new index named `.siem-signals-default-0000002` like so:
<img width="1242" alt="Screen Shot 2020-11-30 at 1 57 12 PM" src="https://user-images.githubusercontent.com/1151048/100684145-fcf38380-3336-11eb-8fe1-b29d3462a1ae.png">

This index is not being cleaned up and when the next time we do an es_archive load we are marking two indexes as being writable


file: x-pack/test/security_solution_cypress/es_archives/alerts/mappings.json
```ts
".siem-signals-default": {
  "is_write_index": true
  }
},
"index": ".siem-signals-default-000001",
```

which leads to the stack trace and the Cypress errors on the front end:

```ts
MacBook-Pro.local] path: /.siem-signals-default-000001, params: {index=.siem-signals-default-000001}
     │      java.lang.IllegalStateException: alias [.siem-signals-default] has more than one write index [.siem-signals-default-000002,.siem-signals-default-000001]
     │          at org.elasticsearch.cluster.metadata.IndexAbstraction$Alias.computeAndValidateAliasProperties(IndexAbstraction.java:276) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
```

This fixes that by updating the mapping and migration number. Each time we migrate/change the signals mapping we will have to perform a PR like this to update each location.

At the moment this is 5 different locations we have to update with the latest mappings.

### Checklist

- [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
FrankHassanabad added a commit that referenced this pull request Dec 8, 2020
…latest signals mapping (#84600) (#85209)

## Summary

Fixes issues within Cypress whenever developers would re-run tests they could become blocked as some of the tests cause a migration/roll over of the signals.
 
To reproduce the error off of master:

Start Cypress 
```ts
cd ./kibana/x-pack/plugins/security_solution
yarn cypress:open-as-ci
```

Then run the export test twice. The first time it will run ok. The second time the test will refuse to run.

<img width="585" alt="Screen Shot 2020-11-30 at 1 57 24 PM" src="https://user-images.githubusercontent.com/1151048/100683706-2233c200-3336-11eb-949f-48f86e884d8b.png">

The second time that you run the test you will get these errors from Cypress and will be blocked until you do workarounds such as logging into `https://localhost:5620` and manually changing the indexes manually or restarting Cypress altogether.
<img width="1067" alt="Screen Shot 2020-11-30 at 6 05 07 PM" src="https://user-images.githubusercontent.com/1151048/100683945-9a9a8300-3336-11eb-9d49-187dee4fc1dc.png">

What is going on with the errors is that a migration is occurring since the existing signals mapping for the tests are not the newer migrated version and these are creating a new index named `.siem-signals-default-0000002` like so:
<img width="1242" alt="Screen Shot 2020-11-30 at 1 57 12 PM" src="https://user-images.githubusercontent.com/1151048/100684145-fcf38380-3336-11eb-8fe1-b29d3462a1ae.png">

This index is not being cleaned up and when the next time we do an es_archive load we are marking two indexes as being writable


file: x-pack/test/security_solution_cypress/es_archives/alerts/mappings.json
```ts
".siem-signals-default": {
  "is_write_index": true
  }
},
"index": ".siem-signals-default-000001",
```

which leads to the stack trace and the Cypress errors on the front end:

```ts
MacBook-Pro.local] path: /.siem-signals-default-000001, params: {index=.siem-signals-default-000001}
     │      java.lang.IllegalStateException: alias [.siem-signals-default] has more than one write index [.siem-signals-default-000002,.siem-signals-default-000001]
     │          at org.elasticsearch.cluster.metadata.IndexAbstraction$Alias.computeAndValidateAliasProperties(IndexAbstraction.java:276) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
```

This fixes that by updating the mapping and migration number. Each time we migrate/change the signals mapping we will have to perform a PR like this to update each location.

At the moment this is 5 different locations we have to update with the latest mappings.

### Checklist

- [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

Co-authored-by: Kibana Machine <[email protected]>
jloleysens added a commit to jloleysens/kibana that referenced this pull request Dec 8, 2020
…or-disable-searchable-snapshot-fields

* 'master' of github.com:elastic/kibana: (29 commits)
  HTTP CRUD+ API for Index Patterns (elastic#83576)
  Don't list packages in devDependencies already listed in dependencies (elastic#85120)
  Remove unused devDependencies (elastic#85121)
  [ILM] Reposition form toggles (elastic#85143)
  [APM] Make sure jest script can be run from anywhere (elastic#85111)
  Add EuiButtonEmptyTo components (elastic#85213)
  skip flaky suite (elastic#85216)
  skip flaky suite (elastic#83775)
  skip flaky suite (elastic#83774)
  skip flaky suite (elastic#83773)
  skip flaky suite (elastic#83793)
  skip flaky suite (elastic#85215)
  skip flaky suite (elastic#85217)
  [Usage Collection] Kibana Overview Page UI Metrics (elastic#81937)
  [Alerting UI] Replaced AlertsContextProvider with KibanaContextProvider and exposed components in API (elastic#84604)
  skip flaky suite (elastic#85208)
  [Security Solutions][Detection Engine] Fixes cypress errors by using latest signals mapping (elastic#84600)
  Small fixes to Kibana search bar (elastic#85079)
  Change link text to say Fleet (elastic#85083)
  [Metrics UI] Refactor Process List API to fetch only top processes (elastic#84716)
  ...

# Conflicts:
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/hot_phase/hot_phase.tsx
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/shared_fields/searchable_snapshot_field/searchable_snapshot_field.tsx
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Detection Alerts Security Solution Detection Alerts Feature release_note:skip Skip the PR/issue when compiling release notes Team:Detections and Resp Security Detection Response Team v7.11.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants