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

Volume device type #276

Merged
merged 7 commits into from
Aug 10, 2022
Merged

Volume device type #276

merged 7 commits into from
Aug 10, 2022

Conversation

ferullo
Copy link
Contributor

@ferullo ferullo commented Aug 1, 2022

Change Summary

This adds volume_device_type.

Sample values

Sample document:

None

Release Target

Ideally for 8.4

Q/A

N/A

For mapping changes:

  • I ran make after making the schema changes, and committed all changes
  • If these field(s) are "exception"-able, I made a companion PR to Kibana adding it (see Readme)
  • If this is a metadata change, I also updated both transform destination schemas to match

For Transform changes:

  • The new transform successfully starts in Kibana
  • The corresponding transform destination schema was updated if necessary

- name: Ext.device.volume_device_type
level: custom
type: keyword
short: FILL ME IN
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Trinity2019 can you update these FILL ME INs

@elasticmachine
Copy link
Contributor

elasticmachine commented Aug 1, 2022

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2022-08-10T19:07:12.015+0000

  • Duration: 7 min 25 sec

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

custom_schemas/custom_dll.yml Outdated Show resolved Hide resolved
custom_schemas/custom_dll.yml Outdated Show resolved Hide resolved
custom_schemas/custom_file.yml Outdated Show resolved Hide resolved
custom_schemas/custom_process.yml Outdated Show resolved Hide resolved
custom_schemas/custom_file.yml Outdated Show resolved Hide resolved
schemas/v1/library/library.yaml Outdated Show resolved Hide resolved
schemas/v1/file/file.yaml Outdated Show resolved Hide resolved
schemas/v1/alerts/ransomware_event.yaml Outdated Show resolved Hide resolved
schemas/v1/alerts/memory_protection_event.yaml Outdated Show resolved Hide resolved
schemas/v1/alerts/memory_protection_event.yaml Outdated Show resolved Hide resolved
@Trinity2019
Copy link
Contributor

Thanks @ferullo for updating endpoint-package!

ferullo and others added 2 commits August 1, 2022 13:43
Co-authored-by: Yamin Tian <56367679+Trinity2019@users.noreply.github.com>
@ferullo
Copy link
Contributor Author

ferullo commented Aug 1, 2022

Thanks for the doc updates @Trinity2019 . For future reference, I should have mentioned that only the files in custom_schemas/ needed to be updated. The rest are auto generated files (which nevertheless are suppose to be committed).

@ferullo
Copy link
Contributor Author

ferullo commented Aug 2, 2022

@elastic/security-onboarding-and-lifecycle-mgt can this be merged or is there a merge-freeze right now?

@kevinlog
Copy link
Contributor

kevinlog commented Aug 2, 2022

@ferullo we have just cut the Endpoint package over for 8.4.0 and released it. Is this critical for 8.4.0 or can it wait for the following release?

Ideally, we start to enforce merge-freezes a couple days before stack FF so that we have time to release the package and ensure there aren't any testing issues.

@pzl can correct me if I'm wrong, we can release another package for 8.4.0, but we'll need to increment the version and re-release the package.

@ferullo
Copy link
Contributor Author

ferullo commented Aug 2, 2022

These fields will be generated by Endpoint in 8.4 but it won't cause any problems if we don't merge this. Waiting for the next package release is ok.

@pzl
Copy link
Member

pzl commented Aug 2, 2022

When this gets in, we can backport for an 8.4.1

@ferullo
Copy link
Contributor Author

ferullo commented Aug 10, 2022

@pzl can this be merged and included in 8.4.1 along with #280 ?

@pzl
Copy link
Member

pzl commented Aug 10, 2022

@ferullo

Yes, this and #280 will be 8.4.1 release as soon as we get them both in.

For this PR, can we update the test files with values for this field? Looks like that should be:

  • package/endpoint/data_stream/alerts/sample_event.json adding:
    • threat.enrichments.indicator.file.Ext.device.volume_device_type
    • threat.indicator.file.Ext.device.volume_device_type
  • package/endpoint/data_stream/file/sample_event.json adding:
    • file.Ext.device.volume_device_type
  • package/endpoint/data_stream/library/sample_event.json adding:
    • dll.Ext.device.volume_device_type
  • package/endpoint/data_stream/process/sample_event.json adding:
    • process.Ext.device.volume_device_type

Once we have values in the test files, this is good to merge

@ferullo ferullo requested a review from a team as a code owner August 10, 2022 16:57
@ferullo
Copy link
Contributor Author

ferullo commented Aug 10, 2022

@Trinity2019 This PR doesn't have file.Ext.device.volume_device_type appear in alert documents. Is it correct to leave it out of alerts? Endpoint testing passes without it in alerts so on the one hand it must not be needed. But I'm not sure if that is because of a quirk in Endpoint testing or if it really never would appear in alerts.

@pzl
Copy link
Member

pzl commented Aug 10, 2022

I believe the CI failure is just the newlines at the end of the sample_event.json files.

@ferullo
Copy link
Contributor Author

ferullo commented Aug 10, 2022

Ah thanks, fixed. I did not make the change you suggested for package/endpoint/data_stream/alerts/sample_event.json. I'm not sure why this new field is appearing in that doc under threat_intel. I assume there is some * used that is pulling in all fields, it isn't intentionally added under threat_intel.

@ferullo
Copy link
Contributor Author

ferullo commented Aug 10, 2022

@pzl are you ok with this being merged?

Copy link
Member

@pzl pzl left a comment

Choose a reason for hiding this comment

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

looks good. ok to merge. I will issue a followup PR that will remove

  • threat.enrichments.indicator.file.Ext.device.volume_device_type
  • threat.indicator.file.Ext.device.volume_device_type

from the alerts index before we ship anywhere

@ferullo
Copy link
Contributor Author

ferullo commented Aug 10, 2022

OK, thanks @pzl!

@ferullo ferullo merged commit 277ecd5 into master Aug 10, 2022
@ferullo ferullo deleted the volume_device_type branch August 10, 2022 20:48
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.

5 participants