-
Notifications
You must be signed in to change notification settings - Fork 497
[microsoft_defender_cloud] Add assessment datastream support for the Cloud Detection and Response (CDR) workflow #15290
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
[microsoft_defender_cloud] Add assessment datastream support for the Cloud Detection and Response (CDR) workflow #15290
Conversation
Pinging @elastic/security-service-integrations (Team:Security-Service Integrations) |
🚀 Benchmarks reportTo see the full report comment with |
@@ -0,0 +1,197 @@ | |||
config_version: 2 | |||
interval: {{interval}} |
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.
Currently, we ask users to provide an interval for data fetching.
@maxcold, should we hardcode it to 24h
? Because it's a full sync approach and we've set the retention period to 24h
in the transform.
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.
The easiest would be to set to 24h. Smaller periods (eg. 12h) would work with the downside that deleted resources will still show up for 24h anyway. Larger period won't really work. So if we can't implement simple validation on the input, I'd say hardcoding to 24h makes sense
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.
Smaller periods (eg. 12h) would work with the downside that deleted resources will still show up for 24h anyway
Right.
Hardcoding it to 24 hours makes sense, instead of keeping it user-configurable.
retention_policy: | ||
time: | ||
field: '@timestamp' | ||
max_age: 24h |
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.
The interval for fetching data and the retention period in the transform are both set to 24 hours.
Since we have limited data in the instance, it still takes 5-6 minutes to fetch all of it.
@maxcold, Do you think there's a chance that sometimes the destination indices might not have any data, or might have missing or incomplete data? Should we include a grace period in the transform to accommodate this?
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.
what exactly do you mean by grace period on the transform? Can you give an example?
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.
Example:
On the first day, data collection starts (let's say at 12:00) and completes after 10 minutes, with the transformed data populated accordingly. Now, on the second day, data collection will start at 12:10. At that time, in the destination indices, due to the retention period surpassing 24 hours in the transform, we might have missing data for those few minutes. To cover that gap, we can extend the retention period from 24 hours to 25 hours (so, grace period of 1hr).
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.
got it, we can change to 26h, I just realised that what we have for our native integration, probably due to the same reason
packages/microsoft_defender_cloud/data_stream/assessment/agent/stream/cel.yml.hbs
Outdated
Show resolved
Hide resolved
packages/microsoft_defender_cloud/data_stream/assessment/agent/stream/cel.yml.hbs
Outdated
Show resolved
Hide resolved
...es/microsoft_defender_cloud/data_stream/assessment/elasticsearch/ingest_pipeline/default.yml
Show resolved
Hide resolved
...es/microsoft_defender_cloud/data_stream/assessment/elasticsearch/ingest_pipeline/default.yml
Show resolved
Hide resolved
…red permission for azure application
packages/microsoft_defender_cloud/elasticsearch/transform/misconfiguration/fields/package.yml
Outdated
Show resolved
Hide resolved
packages/microsoft_defender_cloud/elasticsearch/transform/vulnerability/transform.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.
tested in the qa cluster, findings are being displayed correctly
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.
LGTM pending resolution of @kcreddy's concerns.
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.
LGTM. Will approve after the discussion: #15290 (comment)
|
💚 Build Succeeded
History
|
Package microsoft_defender_cloud - 3.0.0 containing this change is available at https://epr.elastic.co/package/microsoft_defender_cloud/3.0.0/ |
Proposed commit message
Note
To Reviewers:
Checklist
changelog.yml
file.How to test this PR locally
Related issues