-
Notifications
You must be signed in to change notification settings - Fork 444
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
[fortinet_fortigate] add hostname parsing for syslog #11678
base: main
Are you sure you want to change the base?
Conversation
💚 CLA has been signed |
Pinging @elastic/sec-deployment-and-devices (Team:Security-Deployment and Devices) |
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.
Some minor changes, but otherwise seems good to me.
The top-level manifest version needs to be updated as well:
manifest.yml
:
- version: "1.26.0"
+ version: "1.27.0"
packages/fortinet_fortigate/data_stream/log/elasticsearch/ingest_pipeline/default.yml
Outdated
Show resolved
Hide resolved
packages/fortinet_fortigate/data_stream/log/elasticsearch/ingest_pipeline/default.yml
Outdated
Show resolved
Hide resolved
packages/fortinet_fortigate/data_stream/log/elasticsearch/ingest_pipeline/default.yml
Outdated
Show resolved
Hide resolved
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 see the changes adequate, thanks.
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.
In case I wasn't clear, the changes I requested are ones I'm requiring.
- For how we use semver, any additional fields added require at least a
minor
change to versioning. Patch versions are reserved for bugfixes/enhancements that don't alter the structure of documents, among other things. - We don't need to set fields from a grok only to not use them and only remove them later. This just adds unnecessary actions to the pipeline.
…st_pipeline/default.yml Co-authored-by: Taylor Swanson <[email protected]>
Co-authored-by: Taylor Swanson <[email protected]>
…st_pipeline/default.yml Co-authored-by: Taylor Swanson <[email protected]>
…st_pipeline/default.yml Co-authored-by: Taylor Swanson <[email protected]>
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.
Updated changes LGTM, we just need to bump the version in the manifest.
packages/fortinet_fortigate/manifest.yml
- version: "1.26.0"
+ version: "1.27.0"
Proposed commit message
The current parser is not processing the hostname field of syslog messages, so in some cases, the
observer.name
field remains empty.Checklist
changelog.yml
file.