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

Threat implementation reuse correction #1604

Merged
merged 14 commits into from
Sep 16, 2021
Merged

Threat implementation reuse correction #1604

merged 14 commits into from
Sep 16, 2021

Conversation

kgeller
Copy link
Contributor

@kgeller kgeller commented Sep 8, 2021

In the debugging of another issue, I noticed a discrepancy in the implementation of an RFC.

Here under Hash, you'll see we intend to reuse under file and process, but we implemented as reuse under threat.indicator.

@kgeller kgeller self-assigned this Sep 8, 2021
@kgeller kgeller requested review from epixa and rylnd September 8, 2021 20:10
@rylnd
Copy link
Contributor

rylnd commented Sep 8, 2021

@kgeller I think the deletions make sense, but I was also expecting to see the addition of hash fields to threat.indicator.file and threat.enrichments.indicator.file; is that going to be done here or in a separate PR?

@kgeller
Copy link
Contributor Author

kgeller commented Sep 8, 2021

@kgeller I think the deletions make sense, but I was also expecting to see the addition of hash fields to threat.indicator.file and threat.enrichments.indicator.file; is that going to be done here or in a separate PR?

@rylnd Separate PR, just to keep it a little more organized. See #1603

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.

Deletions here LGTM! We have #1603 for the necessary additions, as well.

One related issue I noticed: we may have the same problem with the pe fields, where those are typically nested under indicator.file and not as top-level indicator.pe fields, as the schema currently suggests. We can pick that up in another PR as needed, though.

:shipit: !

Copy link
Contributor

@epixa epixa left a comment

Choose a reason for hiding this comment

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

LGTM

@kgeller kgeller merged commit e27e35b into elastic:master Sep 16, 2021
@kgeller kgeller deleted the rfc-0008-nesting-fix branch September 16, 2021 18:26
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.

None yet

3 participants