-
Notifications
You must be signed in to change notification settings - Fork 419
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
ECS software packages and runtime dependencies #532
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great work, thanks @simitt!
Left a bunch of comments, sometimes calling out to other folks for opinions. But overall I love the way this looks :-)
Perhaps we could also mention in the field set description that package URLs (the package's specific URL, not the repo's) can be recorded in url.original
, when available.
Great idea to get this into ECS. @andrewkroh you might be able to chime in here as I think auditbeat also collects some of this info? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for starting this! Left a few comments.
Two fields I wish I had added for Auditbeat's system/package
that could also be relevant here:
- Package type or origin. Values: RPM, DPKG, Homebrew, NPM, Rubygem, etc.
- Path (where the package is located): e.g.
/usr/local/Cellar/go/1.10.3/
@cwurm how about adding
I like the idea, will add it. |
I'm not sure I have a strong opinion, but I think |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think there should be a release
field for release number used in Redhat packaging? Or would that be too specific to one packaging type.
@andrewkroh Can you give an example of such a release string? Is this meant to support their extra long version strings, where they mix the original package version + their backporting patches on top? |
schemas/package.yml
Outdated
level: core | ||
type: keyword | ||
description: URL from where the package was installed. | ||
example: https://docs.docker.com/compose/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another thing about the URL, I would give an example of a full path to the package archive, all the way to the filename. E.g. "https://artifacts.elastic.co/downloads/elasticsearch/elasticsearch-7.3.1-darwin-x86_64.tar.gz"
If places like Docker Hub don't provide that, perhaps that's acceptable. But I expect this URL to be complete as often as possible. So let's lead by example:
. Pun intended ;-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After the thread here, this might now refer to the "generic URL" of the package and not the URL where it's installed from (what was remote_repository
when this PR started). If so, the description should be updated to say something neutral like "URL of this package" / "URL with more information about this package".
On where the package was installed from - I don't know any data source (DPKG, RPM, Homebrew, Chocolatey) that provides this? So I would rather keep that out of ECS, and we can always add it later when we have a concrete example and need to add it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, if the exact package source URL isn't available, I'll defer to you here.
But I agree we could perhaps flesh out the description of the field a little more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was initially misunderstanding the purpose of this field. Its description is not clear enough, and the example is misleading (Docker also has a registry of packages in Docker Hub).
Please remove the field from this PR, and reopen a PR with just this field.
When reopening the new PR, please adjust the following things to remove the ambiguity:
- clarify the description, perhaps something like "Main URL of the software included in the package"
- change the example so it's aligned with the other examples in the package field set. In other words, this should be Golang's URL, which according to Homebrew, is simply "https://golang.org".
Another discussion point that I'd like to hash out: instead of partially nesting the URL field set here, I think we could use another pattern we've started using elsewhere, and name the field .reference
. WDYT?
2.6.32-754.el6 In this example |
I personally think it would be too specific. Only RPM has it. DPKG has a similar one though, called My preference would be to keep ECS simple and not include it at all, with users always welcome to add extra fields that are useful to them (but are not required or even expected). But if we want to have it, I'd rather have something that covers RPM, DPKG, and anything else (and maybe that's |
On the distro package versions, do rpm and dpkg give easy access to the origin/upstream version number as well? In other words, to get the If extra work is required to parse out the origin version number, it will get nasty real quick. E.g. what if some origin packages contain So I'm wondering if we need to specify this at all, at this time. Perhaps just having If on the other hand we can get the origin version easily from rpm/dpkg, I'd be open to discuss the addition of a second field, for distro's release version as well. |
@andrewkroh have you seen the suggested |
* Move to extended for all fields * Rename url.original to url.full per comment * Remove remote_repository for now as per discussion * Add note how to fill values for license
I think |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a few small changes to request, to package.type
, package.checksum
and package.license
.
The fields package.detailed_version
and package.url.full
require more discussion. Please remove them from this PR for now, and submit a new PR for each.
This way we can can get most of "package" merged in short order, and hash out the details of the other two fields in the new PRs.
schemas/package.yml
Outdated
level: core | ||
type: keyword | ||
description: URL from where the package was installed. | ||
example: https://docs.docker.com/compose/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was initially misunderstanding the purpose of this field. Its description is not clear enough, and the example is misleading (Docker also has a registry of packages in Docker Hub).
Please remove the field from this PR, and reopen a PR with just this field.
When reopening the new PR, please adjust the following things to remove the ambiguity:
- clarify the description, perhaps something like "Main URL of the software included in the package"
- change the example so it's aligned with the other examples in the package field set. In other words, this should be Golang's URL, which according to Homebrew, is simply "https://golang.org".
Another discussion point that I'd like to hash out: instead of partially nesting the URL field set here, I think we could use another pattern we've started using elsewhere, and name the field .reference
. WDYT?
I incorporated all the feedback, from my perspective we need to come to a conclusion in the following points before merging: @webmat I don't have strong opinions about #532 (comment), please let me know how you prefer to move forward. @cwurm I gave the |
I struggle with that - I'm not sure we could always fill it with a sensible value. What would the value for a Homebrew package be? When installed, it's just a folder in |
@simitt Thanks for all of the adjustments! It looks like |
Note: Auditbeat will benefit from the package field set, and currently doesn't seem to capture the package type as an attribute (based on events from a Debian host). |
@webmat I removed |
schemas/package.yml
Outdated
level: extended | ||
type: keyword | ||
description: Package architecture. | ||
example: runtime |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
x86_64
Thanks for all the last minute adjustments 🙂 |
I think without the package type (deb, rpm, homebrew, etc.) it's hard to distinguish between different packages? For example, there is an Also, pretty much every package manager has a URL/homepage for the package (e.g. Do we have a need to merge this now (i.e. is something waiting and ready to implement these fields), or can we wait and get these useful fields in? |
@cwurm As far as I can tell, PR as it stands covers all of what Auditbeat captures about packages, except for entity_id. Is the absence of a place for the URL a blocker for getting this in 1.2?
|
@webmat That the package dataset does not have a |
Initial draft for getting the process started to define ECS fields for installed software packages / runtime dependencies.
adressing #515