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] getDataViewStateFromIndexFields was using wrong type as part of a cast #158594

Merged

Conversation

kqualters-elastic
Copy link
Contributor

@kqualters-elastic kqualters-elastic commented May 26, 2023

Summary

Fixes an issue with the field browser where all types currently display as unkown, this was because in a code path where a type cast happens, we were using the wrong type. To see this, remove the as unknown from the cast, and the typescript compiler will show the problem:

'BrowserField' is deprecated.ts(6385)
index.ts(70, 4): The declaration was marked as deprecated here.
Conversion of type 'DataViewField' to type 'BrowserField' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
  Type 'DataViewField' is missing the following properties from type 'BrowserField': category, description, example, fields, and 2 more.ts(2352)

DataViewField actually only has spec and kbnFieldType properties, spec is of type FieldSpec which is basically the same type as BrowserField, and has sufficient overlap for the (still unsafe, but more safe than as unknown) cast to occur.

Before:
image

After:
image

Copy link
Contributor

@michaelolo24 michaelolo24 left a comment

Choose a reason for hiding this comment

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

Nice work! Pulled down and tested, LGTM

@kqualters-elastic kqualters-elastic requested a review from a team as a code owner May 30, 2023 15:21
@michaelolo24 michaelolo24 added the bug Fixes for quality problems that affect the customer experience label May 30, 2023
@kqualters-elastic kqualters-elastic requested a review from a team as a code owner May 30, 2023 19:19
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Security Solution Tests #7 / Inspect Explore pages "before all" hook for "inspect Hosts page"

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
timelines 214 216 +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 9.2MB 9.2MB -408.0B

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
timelines 20 21 +1

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
securitySolution 61.9KB 62.0KB +139.0B
Unknown metric groups

API count

id before after diff
timelines 257 259 +2

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
securitySolution 416 420 +4
total +6

References to deprecated APIs

id before after diff
securitySolution 608 606 -2

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
securitySolution 500 504 +4
total +6

History

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

cc @kqualters-elastic

Copy link
Contributor

@angorayc angorayc left a comment

Choose a reason for hiding this comment

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

Tested locally, LGTM, thank you Kevin.

@kqualters-elastic kqualters-elastic merged commit 1c75903 into elastic:main May 31, 2023
@kibanamachine
Copy link
Contributor

💔 All backports failed

Status Branch Result
8.8 Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 158594

Questions ?

Please refer to the Backport tool documentation

@kqualters-elastic
Copy link
Contributor Author

💚 All backports created successfully

Status Branch Result
8.8

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

Questions ?

Please refer to the Backport tool documentation

kqualters-elastic added a commit to kqualters-elastic/kibana that referenced this pull request May 31, 2023
…ype as part of a cast (elastic#158594)

## Summary

Fixes an issue with the field browser where all types currently display
as unkown, this was because in a code path where a type cast happens, we
were using the wrong type. To see this, remove the as unknown from the
cast, and the typescript compiler will show the problem:
```
'BrowserField' is deprecated.ts(6385)
index.ts(70, 4): The declaration was marked as deprecated here.
Conversion of type 'DataViewField' to type 'BrowserField' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
  Type 'DataViewField' is missing the following properties from type 'BrowserField': category, description, example, fields, and 2 more.ts(2352)
```
DataViewField actually only has spec and kbnFieldType properties, spec
is of type FieldSpec which is basically the same type as BrowserField,
and has sufficient overlap for the (still unsafe, but more safe than as
unknown) cast to occur.

Before:
<img width="338" alt="image"
src="https://github.com/elastic/kibana/assets/56408403/f31c1f9e-25f0-41ee-9e1c-a70171e41d29">

After:
<img width="555" alt="image"
src="https://github.com/elastic/kibana/assets/56408403/8b462477-2dce-41bb-9592-f34b20634b84">

---------

Co-authored-by: kibanamachine <[email protected]>
(cherry picked from commit 1c75903)

# Conflicts:
#	x-pack/plugins/security_solution/public/detections/components/alerts_table/alerts_grouping.tsx
#	x-pack/plugins/security_solution/tsconfig.json
kqualters-elastic added a commit that referenced this pull request Jun 1, 2023
…rong type as part of a cast (#158594) (#158784)

# Backport

This will backport the following commits from `main` to `8.8`:
- [[Security Solution] getDataViewStateFromIndexFields was using wrong
type as part of a cast
(#158594)](#158594)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Kevin
Qualters","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-05-31T21:13:36Z","message":"[Security
Solution] getDataViewStateFromIndexFields was using wrong type as part
of a cast (#158594)\n\n## Summary\r\n\r\nFixes an issue with the field
browser where all types currently display\r\nas unkown, this was because
in a code path where a type cast happens, we\r\nwere using the wrong
type. To see this, remove the as unknown from the\r\ncast, and the
typescript compiler will show the problem:\r\n```\r\n'BrowserField' is
deprecated.ts(6385)\r\nindex.ts(70, 4): The declaration was marked as
deprecated here.\r\nConversion of type 'DataViewField' to type
'BrowserField' may be a mistake because neither type sufficiently
overlaps with the other. If this was intentional, convert the expression
to 'unknown' first.\r\n Type 'DataViewField' is missing the following
properties from type 'BrowserField': category, description, example,
fields, and 2 more.ts(2352)\r\n```\r\nDataViewField actually only has
spec and kbnFieldType properties, spec\r\nis of type FieldSpec which is
basically the same type as BrowserField,\r\nand has sufficient overlap
for the (still unsafe, but more safe than as\r\nunknown) cast to
occur.\r\n\r\nBefore:\r\n<img width=\"338\"
alt=\"image\"\r\nsrc=\"https://github.com/elastic/kibana/assets/56408403/f31c1f9e-25f0-41ee-9e1c-a70171e41d29\">\r\n\r\n\r\nAfter:\r\n<img
width=\"555\"
alt=\"image\"\r\nsrc=\"https://github.com/elastic/kibana/assets/56408403/8b462477-2dce-41bb-9592-f34b20634b84\">\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"1c75903f92b639e2dcffe76ed8b4ef4d6db3b70d","branchLabelMapping":{"^v8.9.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:fix","Team:Threat
Hunting:Investigations","v8.9.0","v8.8.1"],"number":158594,"url":"https://github.com/elastic/kibana/pull/158594","mergeCommit":{"message":"[Security
Solution] getDataViewStateFromIndexFields was using wrong type as part
of a cast (#158594)\n\n## Summary\r\n\r\nFixes an issue with the field
browser where all types currently display\r\nas unkown, this was because
in a code path where a type cast happens, we\r\nwere using the wrong
type. To see this, remove the as unknown from the\r\ncast, and the
typescript compiler will show the problem:\r\n```\r\n'BrowserField' is
deprecated.ts(6385)\r\nindex.ts(70, 4): The declaration was marked as
deprecated here.\r\nConversion of type 'DataViewField' to type
'BrowserField' may be a mistake because neither type sufficiently
overlaps with the other. If this was intentional, convert the expression
to 'unknown' first.\r\n Type 'DataViewField' is missing the following
properties from type 'BrowserField': category, description, example,
fields, and 2 more.ts(2352)\r\n```\r\nDataViewField actually only has
spec and kbnFieldType properties, spec\r\nis of type FieldSpec which is
basically the same type as BrowserField,\r\nand has sufficient overlap
for the (still unsafe, but more safe than as\r\nunknown) cast to
occur.\r\n\r\nBefore:\r\n<img width=\"338\"
alt=\"image\"\r\nsrc=\"https://github.com/elastic/kibana/assets/56408403/f31c1f9e-25f0-41ee-9e1c-a70171e41d29\">\r\n\r\n\r\nAfter:\r\n<img
width=\"555\"
alt=\"image\"\r\nsrc=\"https://github.com/elastic/kibana/assets/56408403/8b462477-2dce-41bb-9592-f34b20634b84\">\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"1c75903f92b639e2dcffe76ed8b4ef4d6db3b70d"}},"sourceBranch":"main","suggestedTargetBranches":["8.8"],"targetPullRequestStates":[{"branch":"main","label":"v8.9.0","labelRegex":"^v8.9.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/158594","number":158594,"mergeCommit":{"message":"[Security
Solution] getDataViewStateFromIndexFields was using wrong type as part
of a cast (#158594)\n\n## Summary\r\n\r\nFixes an issue with the field
browser where all types currently display\r\nas unkown, this was because
in a code path where a type cast happens, we\r\nwere using the wrong
type. To see this, remove the as unknown from the\r\ncast, and the
typescript compiler will show the problem:\r\n```\r\n'BrowserField' is
deprecated.ts(6385)\r\nindex.ts(70, 4): The declaration was marked as
deprecated here.\r\nConversion of type 'DataViewField' to type
'BrowserField' may be a mistake because neither type sufficiently
overlaps with the other. If this was intentional, convert the expression
to 'unknown' first.\r\n Type 'DataViewField' is missing the following
properties from type 'BrowserField': category, description, example,
fields, and 2 more.ts(2352)\r\n```\r\nDataViewField actually only has
spec and kbnFieldType properties, spec\r\nis of type FieldSpec which is
basically the same type as BrowserField,\r\nand has sufficient overlap
for the (still unsafe, but more safe than as\r\nunknown) cast to
occur.\r\n\r\nBefore:\r\n<img width=\"338\"
alt=\"image\"\r\nsrc=\"https://github.com/elastic/kibana/assets/56408403/f31c1f9e-25f0-41ee-9e1c-a70171e41d29\">\r\n\r\n\r\nAfter:\r\n<img
width=\"555\"
alt=\"image\"\r\nsrc=\"https://github.com/elastic/kibana/assets/56408403/8b462477-2dce-41bb-9592-f34b20634b84\">\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"1c75903f92b639e2dcffe76ed8b4ef4d6db3b70d"}},{"branch":"8.8","label":"v8.8.1","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

---------

Co-authored-by: kibanamachine <[email protected]>
@michaelolo24 michaelolo24 added the Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. label Jun 2, 2023
@elasticmachine
Copy link
Contributor

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

@michaelolo24 michaelolo24 removed the bug Fixes for quality problems that affect the customer experience label Jun 6, 2023
dhurley14 added a commit that referenced this pull request Jun 23, 2023
…jsonc (#160445)

## Summary

Adds missing dependency from this PR:
#158594
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jun 23, 2023
…jsonc (elastic#160445)

## Summary

Adds missing dependency from this PR:
elastic#158594

(cherry picked from commit 1ee60b0)
kibanamachine added a commit that referenced this pull request Jun 23, 2023
…ibana.jsonc (#160445) (#160464)

# Backport

This will backport the following commits from `main` to `8.9`:
- [[Security Solution] adds missing dependency `fieldFormats` in
kibana.jsonc (#160445)](#160445)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Devin W.
Hurley","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-06-23T21:14:17Z","message":"[Security
Solution] adds missing dependency `fieldFormats` in kibana.jsonc
(#160445)\n\n## Summary\r\n\r\nAdds missing dependency from this
PR:\r\nhttps://github.com//pull/158594","sha":"1ee60b0ba85409aaca28d65dc70956b082e66a7a","branchLabelMapping":{"^v8.10.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["review","release_note:skip","v8.9.0","v8.10.0"],"number":160445,"url":"https://github.com/elastic/kibana/pull/160445","mergeCommit":{"message":"[Security
Solution] adds missing dependency `fieldFormats` in kibana.jsonc
(#160445)\n\n## Summary\r\n\r\nAdds missing dependency from this
PR:\r\nhttps://github.com//pull/158594","sha":"1ee60b0ba85409aaca28d65dc70956b082e66a7a"}},"sourceBranch":"main","suggestedTargetBranches":["8.9"],"targetPullRequestStates":[{"branch":"8.9","label":"v8.9.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.10.0","labelRegex":"^v8.10.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/160445","number":160445,"mergeCommit":{"message":"[Security
Solution] adds missing dependency `fieldFormats` in kibana.jsonc
(#160445)\n\n## Summary\r\n\r\nAdds missing dependency from this
PR:\r\nhttps://github.com//pull/158594","sha":"1ee60b0ba85409aaca28d65dc70956b082e66a7a"}}]}]
BACKPORT-->

Co-authored-by: Devin W. Hurley <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixed release_note:fix Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Threat Hunting:Investigations Security Solution Investigations Team v8.8.1 v8.9.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants