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

[New Integration] Menlo Security #9315

Merged
merged 52 commits into from Apr 11, 2024
Merged

[New Integration] Menlo Security #9315

merged 52 commits into from Apr 11, 2024

Conversation

tehbooom
Copy link
Member

@tehbooom tehbooom commented Mar 8, 2024

  • Enhancement

What does this PR do?

This PR adds the Menlo Security integration for Web and DLP logs

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.
  • I have added an entry to my package's changelog.yml file.
  • I have verified that Kibana version constraints are current according to guidelines.

Author's Checklist

  • [ ]

How to test this PR locally

Related issues

Screenshots

Copy link
Contributor

@mrodm mrodm left a comment

Choose a reason for hiding this comment

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

Commenting about general settings, code owner team should also review this package

@@ -0,0 +1,81 @@
format_version: 2.5.1
Copy link
Contributor

Choose a reason for hiding this comment

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

Could it be used here a newer version of package-spec ? At this moment, latest version is 3.1.2.

This would also ensure there are more validation checks performed.

Comment on lines +16 to +17
elastic:
subscription: "basic"
Copy link
Contributor

Choose a reason for hiding this comment

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

Remember to add the capabilities field if this package requires it for Serverless.
For instance, other security packages define this:

Suggested change
elastic:
subscription: "basic"
elastic:
subscription: "basic"
capabilities:
- security

This would make this package available in Serverless just for Security projects (and not for Observability projects). cc @elastic/security-service-integrations

@@ -223,6 +223,7 @@
/packages/m365_defender @elastic/security-service-integrations
/packages/mattermost @elastic/security-service-integrations
/packages/memcached @elastic/obs-infraobs-integrations
/packages/menlo @elastic/security-external-integrations
Copy link
Contributor

Choose a reason for hiding this comment

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

Just double checking, it has been set as a reviewer of this PR @elastic/security-service-integrations but here it is set another team.

Which team should be used as a code owner?

Copy link
Member Author

Choose a reason for hiding this comment

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

I believe this was a mistake on my part selecting the wrong team. The CODEOWNERS is correct

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please change this to @elastic/security-service-integrations?
Our older handle used to be @elastic/security-external-integrations.

@tehbooom tehbooom added the Team:Security-External Integrations Label for the Security External Integrations team label Mar 11, 2024
@elasticmachine
Copy link

Pinging @elastic/security-external-integrations (Team:Security-External Integrations)

@tehbooom tehbooom removed the Team:Security-Service Integrations Security Service Integrations Team label Mar 11, 2024
@jamiehynds jamiehynds added Team:Security-Service Integrations Security Service Integrations Team and removed Team:Security-External Integrations Label for the Security External Integrations team labels Mar 14, 2024
@@ -223,6 +223,7 @@
/packages/m365_defender @elastic/security-service-integrations
/packages/mattermost @elastic/security-service-integrations
/packages/memcached @elastic/obs-infraobs-integrations
/packages/menlo @elastic/security-external-integrations
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please change this to @elastic/security-service-integrations?
Our older handle used to be @elastic/security-external-integrations.

packages/menlo/changelog.yml Outdated Show resolved Hide resolved
packages/menlo/_dev/build/docs/README.md Show resolved Hide resolved
packages/menlo/_dev/deploy/docker/config.yml Outdated Show resolved Hide resolved
"version": "8.11.0"
},
"event": {
"action": "block",
Copy link
Contributor

Choose a reason for hiding this comment

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

Have event.type as denied and event.action as blocked
Reference: https://www.elastic.co/guide/en/ecs/8.11/ecs-allowed-values-event-type.html#ecs-event-type-denied

packages/menlo/manifest.yml Outdated Show resolved Hide resolved
@tehbooom tehbooom requested review from kcreddy, elasticitservice and a team and removed request for elasticitservice April 3, 2024 12:07
Copy link
Contributor

@kcreddy kcreddy left a comment

Choose a reason for hiding this comment

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

Thanks for clearing all the review comments. Just one last thing and we can merge 😄

Comment on lines 24 to 33
- date:
field: "json.event.event_time"
formats:
- "ISO8601"
target_field: "event.created"
timezone: "UTC"
on_failure:
- append:
field: error.message
value: 'Processor {{{_ingest.on_failure_processor_type}}} with tag {{{_ingest.on_failure_processor_tag}}} in pipeline {{{_ingest.pipeline}}} failed with message: {{{_ingest.on_failure_message}}}'
Copy link
Contributor

Choose a reason for hiding this comment

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

When you are populating error.message with {{{_ingest.on_failure_processor_tag}}}, add tag: <tag-name> to the processor definition and also to render {{{_ingest.on_failure_processor_tag}}} inside error.message.

Example: https://github.com/elastic/integrations/blob/main/packages/ti_threatconnect/data_stream/indicator/elasticsearch/ingest_pipeline/default.yml#L77

@elasticmachine
Copy link

💚 Build Succeeded

History

Copy link

@tehbooom tehbooom requested a review from a team April 11, 2024 13:06
Copy link
Contributor

@kcreddy kcreddy left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏼
Dashboards will be a nice addition to this package next.

@kcreddy kcreddy merged commit 8d1d0ad into elastic:main Apr 11, 2024
5 checks passed
@elasticmachine
Copy link

Package menlo - 0.1.0 containing this change is available at https://epr.elastic.co/search?package=menlo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants