-
Notifications
You must be signed in to change notification settings - Fork 87
contextualize findings with additional metadata fields for correlation engine #1561
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
base: main
Are you sure you want to change the base?
Conversation
…n engine Signed-off-by: Subhobrata Dey <[email protected]>
ThreatIntelAlertService threatIntelAlertService = new ThreatIntelAlertService(client, clusterService, xContentRegistry); | ||
SaIoCScanService ioCScanService = new SaIoCScanService(client, clusterService, xContentRegistry, iocFindingService, threatIntelAlertService, notificationService); | ||
DefaultTifSourceConfigLoaderService defaultTifSourceConfigLoaderService = new DefaultTifSourceConfigLoaderService(builtInTIFMetadataLoader, client, saTifSourceConfigManagementService); | ||
QueryShardContextFactory qsqFactory = new QueryShardContextFactory(client, clusterService, scriptService, xContentRegistry, namedWriteableRegistry, environment); |
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.
nit: we should avoid abbreviating the variable name
public void onResponse(List<DocLevelQuery> dlqs) { | ||
try { | ||
List<IndexMonitorRequest> monitorRequests = new ArrayList<>(); | ||
try { |
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.
Let's break this into many private methods. It's nearly impossible to read/understand with this level of nesting
Map<String, String> fieldMappings = ruleFieldMappings.get(category); | ||
queryBackendMap.put(category, new OSQueryBackend(fieldMappings, true, true)); | ||
} | ||
try { |
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.
Same comment on using private methods over nesting
} | ||
|
||
private void handleUpsertWorkflowFailure(final Exception e, final ActionListener<List<IndexMonitorResponse>> listener, | ||
private void fetchCorrelationRuleFields(Detector detector, ActionListener<SearchResponse> listener) { |
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 we have a way to handle this same logic if the correlation rule is created after the detector?
} | ||
index = indexMetadata.getIndex(); | ||
indexSettings = indexMetadata.getSettings(); | ||
return new Triple<>(index, indexSettings, indexMetadata); |
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.
Why do we need to break this into a triple? Returning IndexMetadata only allows the caller access to all 3 elements of the triple
return new Triple<>(index, indexSettings, indexMetadata); | ||
} | ||
|
||
public QueryShardContext createShardContext(String indexName) throws IOException { |
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.
Can you explain what this method is doing and why we need it? I see a lot of dummy values
Description
contextualize findings with additional metadata fields for correlation engine
Related Issues
Resolves #[Issue number to be closed when this PR is merged]
Check List
--signoff
.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.