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

aws.securityhub_findings: Improve support for CDR #11158

Merged
merged 37 commits into from
Oct 30, 2024

Conversation

kcreddy
Copy link
Contributor

@kcreddy kcreddy commented Sep 17, 2024

Proposed commit message

Improve support for CDR.

Fixes: #11040

Note to reviewers: Please DM me for access to the document(s) linked in the issue, it might help in the review.

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.

How to test this PR locally

cd packages/aws && elastic-package stack down && elastic-package build && elastic-package stack up --version=8.14.0 -d -v && eval "$(elastic-package stack shellinit)" && elastic-package test pipeline --generate -v --data-streams=securityhub_findings

Related issues

Screenshots

Screenshot 2024-09-23 at 5 47 15 PM

@kcreddy kcreddy added enhancement New feature or request Team:Security-Service Integrations Security Service Integrations Team [elastic/security-service-integrations] labels Sep 17, 2024
@kcreddy kcreddy self-assigned this Sep 17, 2024
@elastic-vault-github-plugin-prod
Copy link

elastic-vault-github-plugin-prod bot commented Sep 17, 2024

🚀 Benchmarks report

Package aws 👍(8) 💚(7) 💔(4)

Expand to view
Data stream Previous EPS New EPS Diff (%) Result
s3access 4651.16 3831.42 -819.74 (-17.62%) 💔
apigateway_logs 10989.01 4464.29 -6524.72 (-59.37%) 💔
ec2_metrics 25000 17857.14 -7142.86 (-28.57%) 💔
firewall_logs 3300.33 2645.5 -654.83 (-19.84%) 💔

To see the full report comment with /test benchmark fullreport

@kcreddy kcreddy marked this pull request as ready for review September 23, 2024 12:24
@kcreddy kcreddy requested review from a team as code owners September 23, 2024 12:24
@elasticmachine
Copy link

Pinging @elastic/security-service-integrations (Team:Security-Service Integrations)

@kcreddy kcreddy requested a review from maxcold September 23, 2024 12:24
@andrewkroh andrewkroh added the dashboard Relates to a Kibana dashboard bug, enhancement, or modification. label Sep 23, 2024
Copy link
Contributor

@efd6 efd6 left a comment

Choose a reason for hiding this comment

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

The issue refers to a document upload, but I cannot find it. So I cannot see whether this follows what has been designed. Is there a reason this is not a public document in the issue?

@kcreddy kcreddy requested a review from efd6 September 24, 2024 10:44
@kcreddy
Copy link
Contributor Author

kcreddy commented Oct 11, 2024

/test

Copy link
Contributor

@maxcold maxcold left a comment

Choose a reason for hiding this comment

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

A couple of more comments to fix and good to go. Thanks for the patience!

@kcreddy kcreddy requested a review from andrewkroh October 28, 2024 17:48
Comment on lines 36 to 43
- set:
field: observer.vendor
value: AWS Security Hub
tag: set_observer_vendor
- set:
field: cloud.provider
value: aws
tag: set_cloud_provider
Copy link
Member

Choose a reason for hiding this comment

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

The three fields being converted to constant_keyword would all benefit from removal from _source.

I recommend setting the static values in the ecs.yml file where they fields are declared instead of the ingest pipeline, and then exchange the three set processors with a single remove processor that has a description field explaining that the fields are defined as constant_keyword and we are removing the fields from _source to gain storage efficiency.

@andrewkroh
Copy link
Member

Also, please update the commit message (in the PR description) to specify why the minimum kibana version was changed.

@kcreddy kcreddy requested a review from andrewkroh October 29, 2024 06:01
@kcreddy
Copy link
Contributor Author

kcreddy commented Oct 29, 2024

@andrewkroh the comments are addressed in 0e44091 and PR commit message is also updated.

Copy link
Member

@andrewkroh andrewkroh left a comment

Choose a reason for hiding this comment

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

I left a few more minor comments.

Copy link
Member

@shmsr shmsr left a comment

Choose a reason for hiding this comment

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

Based on other approvals, approving as a CODEOWNER from @elastic/obs-infraobs-integrations

Copy link

Quality Gate failed Quality Gate failed

Failed conditions
48.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube

@elasticmachine
Copy link

💚 Build Succeeded

History

cc @kcreddy

@maxcold maxcold merged commit 8a249bc into elastic:main Oct 30, 2024
4 of 5 checks passed
@elastic-vault-github-plugin-prod

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dashboard Relates to a Kibana dashboard bug, enhancement, or modification. enhancement New feature or request Integration:aws AWS Team:Security-Service Integrations Security Service Integrations Team [elastic/security-service-integrations]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

aws.securityhub_findings: Implement mappings for Cloud Security Workflow
9 participants