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

Fix ioc store config mappings #1087

Merged

Conversation

jowg-amazon
Copy link
Collaborator

Description

[Describe what this change achieves]

Issues Resolved

[List any issues this PR will resolve]

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --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.

Signed-off-by: Joanne Wang <jowg@amazon.com>
Signed-off-by: Joanne Wang <jowg@amazon.com>
@@ -115,8 +115,11 @@ public void indexIocs(List<STIX2IOC> iocs) throws IOException {
String feedIndexName = initFeedIndex(saTifSourceConfig.getId());

// Add the created index to the IocStoreConfig
((DefaultIocStoreConfig) saTifSourceConfig.getIocStoreConfig()).getIocMapStore().putIfAbsent(saTifSourceConfig.getId(), new ArrayList<>());
((DefaultIocStoreConfig) saTifSourceConfig.getIocStoreConfig()).getIocMapStore().get(saTifSourceConfig.getId()).add(feedIndexName);
saTifSourceConfig.getIocTypes().forEach(type -> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not blocking, but we should probably considering refactoring the IocTypes list in the config data model to store a list of the IOCType enums instead of Strings.
https://github.com/opensearch-project/security-analytics-commons/blob/main/tif/src/main/java/org/opensearch/securityanalytics/commons/model/IOCType.java#L10

@@ -25,7 +25,7 @@ public class DefaultIocStoreConfig extends IocStoreConfig implements Writeable,
public static final String DEFAULT_FIELD = "default";
public static final String IOC_MAP = "ioc_map";

// Maps the SATIFSourceConfig ID to the list of index/alias names
// Maps the IOCs to the list of index/alias names
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would this be "IOC types" to the index/alias names?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes fixing comment to include ioc types

Signed-off-by: Joanne Wang <jowg@amazon.com>
// // wait for job runner to run
// waitUntil(() -> {
// try {
// return verifyJobRan(createdId, firstUpdatedTime);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not blocking, but could you add a javadoc comment to the verifyJobRan function to help clarify its use?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

added

Signed-off-by: Joanne Wang <jowg@amazon.com>
@@ -115,8 +115,11 @@ public void indexIocs(List<STIX2IOC> iocs) throws IOException {
String feedIndexName = initFeedIndex(saTifSourceConfig.getId());

// Add the created index to the IocStoreConfig
((DefaultIocStoreConfig) saTifSourceConfig.getIocStoreConfig()).getIocMapStore().putIfAbsent(saTifSourceConfig.getId(), new ArrayList<>());
((DefaultIocStoreConfig) saTifSourceConfig.getIocStoreConfig()).getIocMapStore().get(saTifSourceConfig.getId()).add(feedIndexName);
saTifSourceConfig.getIocTypes().forEach(type -> {
Copy link
Collaborator

@AWSHurneyt AWSHurneyt Jun 20, 2024

Choose a reason for hiding this comment

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

@jowg-amazon A quick way to resolve the bug where the index name is being added each time the feed is refreshed would be to add the following add this following check around the forEach loop.

if (!feedIndexExists(feedIndexName)) {
...
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Would this always be true since we are doing initFeedIndex first?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually, it would be better to refactor the initFeedIndex function to take in the index name, and a boolean "exists" variable; and replace the call to feedIndexExists in the init function with the boolean variable.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Refactored method so that it checks if the index exists first, if it does not exist then it will initialize the index and add it to the ioc map

Signed-off-by: Joanne Wang <jowg@amazon.com>
@jowg-amazon jowg-amazon merged commit c28f84f into opensearch-project:feature/threat_intel Jun 20, 2024
2 checks passed
AWSHurneyt pushed a commit to AWSHurneyt/security-analytics that referenced this pull request Jun 25, 2024
* fix mappings

Signed-off-by: Joanne Wang <jowg@amazon.com>

* comment

Signed-off-by: Joanne Wang <jowg@amazon.com>

* fix comment

Signed-off-by: Joanne Wang <jowg@amazon.com>

* added java doc and todo

Signed-off-by: Joanne Wang <jowg@amazon.com>

* remove duplicate index names from mapping

Signed-off-by: Joanne Wang <jowg@amazon.com>

---------

Signed-off-by: Joanne Wang <jowg@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants