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] Removes the elastic legacy client from lists and security_solution plugins #106130

Merged
merged 16 commits into from
Jul 21, 2021

Conversation

FrankHassanabad
Copy link
Contributor

@FrankHassanabad FrankHassanabad commented Jul 19, 2021

Summary

Addressees #83910 by removing the elastic legacy client from:

  • lists plugin
  • security_solution plugin
  • kbn-securitysolution-es-utils package

Removes found dead code in security_solution plugin:

  • server/lib/configuration/inmemory_configuration_adapter.ts
  • server/lib/detection_engine/privileges/read_privileges.ts
  • server/lib/configuration/index.ts
  • server/lib/configuration/adapter_types.ts
  • server/lib/compose/kibana.ts
  • server/lib/ecs_fields/extend_map.test.ts
  • server/lib/ecs_fields/extend_map.ts
  • server/lib/index_fields/elasticsearch_adapter.ts
  • server/lib/index_fields/index.ts
  • server/lib/index_fields/mock.ts
  • server/lib/index_fields/types.ts
  • server/lib/source_status/elasticsearch_adapter.ts
  • server/lib/source_status/index.ts
  • server/lib/source_status/query.dsl.ts
  • server/lib/source_status/types.ts
  • server/lib/sources/configuration.test.ts
  • server/lib/sources/configuration.ts
  • server/lib/sources/index.ts
  • server/lib/sources/types.ts

Removes dead code in lists plugin:

  • server/schemas/common/get_call_cluster.mock.ts
  • server/lib/ecs_fields/index.ts
  • server/lib/framework/kibana_framework_adapter.ts

Removes dead types from security_solution plugin:

  • server/lib/framework/types.ts
  • server/lib/types.ts

Removes dead functions from security_solution plugin:

  • server/utils/build_query/calculate_timeseries_interval.ts
  • server/utils/runtime_types.ts

What to check as a reviewer

  • Ensure that there is no left over words of legacy such as legacy.something
  • Ensure there are no more callAsCurrentUser since that is all dead and gone
  • Ensure anywhere you see esClient.someThing it returns the .body at the end or destructors it as in { body } = esClient.someThing

Risk Matrix

Risk Probability Severity Mitigation/Notes
Telemetry might stop working or have invalid values. Med High We will have to manually test telemetry. Pinged people from telemetry for a code review
An REST route returns invalid values. Med High e2e tests caught some of these already. The rest of the code was re-checked by hand
Deleted function/code might actually be still in use somewhere. Low High e2e and unit tests should catch any of this.

Checklist

Delete any items that are not applicable to this PR.

@FrankHassanabad FrankHassanabad self-assigned this Jul 19, 2021
@FrankHassanabad FrankHassanabad changed the title Remove legacy client [Security Solutions] Removes the elastic legacy client from lists and security_solution plugins Jul 19, 2021
@FrankHassanabad FrankHassanabad added v8.0.0 v7.15.0 release_note:skip Skip the PR/issue when compiling release notes Team:Detections and Resp Security Detection Response Team labels Jul 19, 2021
@FrankHassanabad FrankHassanabad marked this pull request as ready for review July 19, 2021 18:45
@FrankHassanabad FrankHassanabad requested review from a team as code owners July 19, 2021 18:45
@elasticmachine
Copy link
Contributor

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

@FrankHassanabad FrankHassanabad added the auto-backport Deprecated - use backport:version if exact versions are needed label Jul 19, 2021
@FrankHassanabad
Copy link
Contributor Author

@elasticmachine merge upstream

@@ -108,7 +108,9 @@ export class TelemetryDiagTask {
}
this.logger.debug(`Received ${hits.length} diagnostic alerts`);

const diagAlerts: TelemetryEvent[] = hits.map((h) => h._source);
const diagAlerts: TelemetryEvent[] = hits.flatMap((h) =>
Copy link
Contributor

Choose a reason for hiding this comment

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

Good idea

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a bit of a trick where I'm using flatMap to do both filtering and mapping together. It's not the way I would prefer to do it but it fixes our TypeScript issues where TypeScript cannot remove undefined cleanly with a filter

Copy link
Contributor

@pjhampton pjhampton left a comment

Choose a reason for hiding this comment

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

🌔 🚀 ✨ LGTM ✨ 🚀 🌔

Reviewed and tested the security telemetry reactoring. +1 from me

@spong

This comment has been minimized.

@FrankHassanabad

This comment has been minimized.

Copy link
Member

@spong spong left a comment

Choose a reason for hiding this comment

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

Checked out, tested locally, performed code review, and verified the following as requested:

  • Ensure that there is no left over words of legacy such as legacy.something
  • Ensure there are no more callAsCurrentUser since that is all dead and gone
  • Ensure anywhere you see esClient.someThing it returns the .body at the end or destructors it as in { body } = esClient.someThing

LGTM! Thanks for all the additional cleanup of dead code as well! 🚀

FrankHassanabad and others added 4 commits July 20, 2021 14:58
…outes/rules/create_rules_route.ts

Co-authored-by: Garrett Spong <spong@users.noreply.github.com>
…outes/signals/open_close_signals_route.ts

Co-authored-by: Garrett Spong <spong@users.noreply.github.com>
@FrankHassanabad FrankHassanabad enabled auto-merge (squash) July 20, 2021 22:04
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
securitySolution 1131 1130 -1
Unknown metric groups

API count

id before after diff
securitySolution 1182 1181 -1

References to deprecated APIs

id before after diff
lists 239 233 -6
securitySolution 364 359 -5
total -11

History

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

cc @FrankHassanabad

@FrankHassanabad FrankHassanabad merged commit dd8a4a7 into elastic:master Jul 21, 2021
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jul 21, 2021
… security_solution plugins (elastic#106130)

## Summary

Addressees elastic#83910 by removing the elastic legacy client from:
* `lists` plugin
* `security_solution` plugin
* `kbn-securitysolution-es-utils` package

Removes found dead code in `security_solution` plugin:
* `server/lib/configuration/inmemory_configuration_adapter.ts`
* `server/lib/detection_engine/privileges/read_privileges.ts`
* `server/lib/configuration/index.ts`
* `server/lib/configuration/adapter_types.ts`
* `server/lib/compose/kibana.ts`
* `server/lib/ecs_fields/extend_map.test.ts`
* `server/lib/ecs_fields/extend_map.ts`
* `server/lib/index_fields/elasticsearch_adapter.ts`
* `server/lib/index_fields/index.ts`
* `server/lib/index_fields/mock.ts`
* `server/lib/index_fields/types.ts`
* `server/lib/source_status/elasticsearch_adapter.ts`
* `server/lib/source_status/index.ts`
* `server/lib/source_status/query.dsl.ts`
* `server/lib/source_status/types.ts`
* `server/lib/sources/configuration.test.ts`
* `server/lib/sources/configuration.ts`
* `server/lib/sources/index.ts`
* `server/lib/sources/types.ts`

Removes dead code in `lists` plugin:
* `server/schemas/common/get_call_cluster.mock.ts`
* `server/lib/ecs_fields/index.ts`
* `server/lib/framework/kibana_framework_adapter.ts`

Removes dead types from `security_solution` plugin:
* `server/lib/framework/types.ts`
* `server/lib/types.ts`

Removes dead functions from `security_solution` plugin:
* `server/utils/build_query/calculate_timeseries_interval.ts`
* `server/utils/runtime_types.ts`

### What to check as a reviewer
* Ensure that there is no left over words of `legacy` such as `legacy.something`
* Ensure there are no more `callAsCurrentUser` since that is all dead and gone
* Ensure anywhere you see `esClient.someThing` it returns the `.body` at the end or destructors it as in `{ body } = esClient.someThing`


### Risk Matrix

| Risk                      | Probability | Severity | Mitigation/Notes        |
|---------------------------|-------------|----------|-------------------------|
| Telemetry might stop working or have invalid values. | Med | High | We will have to manually test telemetry. Pinged people from telemetry for a code review |
| An REST route returns invalid values. | Med | High | e2e tests caught some of these already. The rest of the code was re-checked by hand |
| Deleted function/code might actually be still in use somewhere. | Low | High | e2e and unit tests should catch any of this. |



### Checklist

Delete any items that are not applicable to this PR.

- [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
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
7.x

This backport PR will be merged automatically after passing CI.

@FrankHassanabad FrankHassanabad deleted the remove-legacy-client branch July 21, 2021 00:18
kibanamachine added a commit that referenced this pull request Jul 21, 2021
… security_solution plugins (#106130) (#106342)

## Summary

Addressees #83910 by removing the elastic legacy client from:
* `lists` plugin
* `security_solution` plugin
* `kbn-securitysolution-es-utils` package

Removes found dead code in `security_solution` plugin:
* `server/lib/configuration/inmemory_configuration_adapter.ts`
* `server/lib/detection_engine/privileges/read_privileges.ts`
* `server/lib/configuration/index.ts`
* `server/lib/configuration/adapter_types.ts`
* `server/lib/compose/kibana.ts`
* `server/lib/ecs_fields/extend_map.test.ts`
* `server/lib/ecs_fields/extend_map.ts`
* `server/lib/index_fields/elasticsearch_adapter.ts`
* `server/lib/index_fields/index.ts`
* `server/lib/index_fields/mock.ts`
* `server/lib/index_fields/types.ts`
* `server/lib/source_status/elasticsearch_adapter.ts`
* `server/lib/source_status/index.ts`
* `server/lib/source_status/query.dsl.ts`
* `server/lib/source_status/types.ts`
* `server/lib/sources/configuration.test.ts`
* `server/lib/sources/configuration.ts`
* `server/lib/sources/index.ts`
* `server/lib/sources/types.ts`

Removes dead code in `lists` plugin:
* `server/schemas/common/get_call_cluster.mock.ts`
* `server/lib/ecs_fields/index.ts`
* `server/lib/framework/kibana_framework_adapter.ts`

Removes dead types from `security_solution` plugin:
* `server/lib/framework/types.ts`
* `server/lib/types.ts`

Removes dead functions from `security_solution` plugin:
* `server/utils/build_query/calculate_timeseries_interval.ts`
* `server/utils/runtime_types.ts`

### What to check as a reviewer
* Ensure that there is no left over words of `legacy` such as `legacy.something`
* Ensure there are no more `callAsCurrentUser` since that is all dead and gone
* Ensure anywhere you see `esClient.someThing` it returns the `.body` at the end or destructors it as in `{ body } = esClient.someThing`


### Risk Matrix

| Risk                      | Probability | Severity | Mitigation/Notes        |
|---------------------------|-------------|----------|-------------------------|
| Telemetry might stop working or have invalid values. | Med | High | We will have to manually test telemetry. Pinged people from telemetry for a code review |
| An REST route returns invalid values. | Med | High | e2e tests caught some of these already. The rest of the code was re-checked by hand |
| Deleted function/code might actually be still in use somewhere. | Low | High | e2e and unit tests should catch any of this. |



### Checklist

Delete any items that are not applicable to this PR.

- [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: Frank Hassanabad <frank.hassanabad@elastic.co>
jloleysens added a commit to jloleysens/kibana that referenced this pull request Jul 21, 2021
…y-show-migrate-to-authzd-users

* 'master' of github.com:elastic/kibana: (48 commits)
  [Canvas] Expression shape (elastic#103219)
  [FTR] Skips Vega tests
  [Sample data] Use Lens in ecommerce data (elastic#106039)
  [APM] Backends inventory & overview page routes (elastic#106223)
  [TSVB] Add more functional tests for Gauge and TopN (elastic#105361)
  Add toggle to enable/disable rule install from SOs (elastic#106189)
  Improve unit test coverage of FS API calls (elastic#106242)
  Remove recursive plugin status in meta field (elastic#106286)
  [Ingest pipelines] add community id processor (elastic#103863)
  [XY axis] Fixes the values inside bar charts (elastic#106198)
  [data.search] Set default expiration to 1m if search sessions are disabled (elastic#105329)
  set the doc title when navigating to reporting and unset when navigating away (elastic#106253)
  [Lens] Display legend inside chart (elastic#105571)
  [RAC] [TGrid] Migrate the TGrid's rendering to `EuiDataGrid` (elastic#106199)
  [Security Solutions] Removes the elastic legacy client from lists and security_solution plugins (elastic#106130)
  [Enterprise Search] Require security plugin in 8.0 (elastic#106307)
  [DOCS] Updates screenshots in Dev Tools docs (elastic#105859)
  [DOCS] Updates text and screenshots in tags doc (elastic#105853)
  [Alerting] Allow rule types to extract/inject saved object references on rule CRU (elastic#101896)
  Jest and Storybook fixes (elastic#104991)
  ...

# Conflicts:
#	x-pack/plugins/reporting/public/plugin.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes Team:Detections and Resp Security Detection Response Team v7.15.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants