-
Notifications
You must be signed in to change notification settings - Fork 415
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
Added RST IoC Lookup connector. Fixes for Report Hub and Threat Feed #2864
base: master
Are you sure you want to change the base?
Conversation
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.
Hi @k1r10n , thank you for your PR and this new connector!
As I wrote in the comments, there is an issue with CONNECTOR_UPDATE_EXISTING_DATA
env var:
- rst-threat-hub doesn't ingest anything as
VALIDATION_ERROR
is raised for each message - rst-threat-feed ingests data and I didn't see any
VALIDATION_ERROR
so far (there are manyMISSING_REFERENCE_ERROR
but it's already the case on master, your fixes/updates are not responsible) - I tested rst-ioc-lookup manually and in auto mode, it seems to working as intended in both ways
FIY, all my tests have been done by running connectors locally with env vars set in config.yml
.
Could you fix the issue in the comments so we can merge your PR please? Thanks 😇
} | ||
self.update_existing_data = get_config_variable( | ||
"CONNECTOR_UPDATE_EXISTING_DATA", | ||
["connector", "update_existing_data"], | ||
config, | ||
True, |
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.
I'm not sure it does what it's supposed to do. True
here refers to isNumber
argument (which parse an environment variable to an integer), but I think self.update_existing_data
needs to be a boolean.
If not, a VALIDATION_ERROR
is raised during ingestion.
"CONNECTOR_UPDATE_EXISTING_DATA", | ||
["connector", "update_existing_data"], | ||
config, | ||
True, |
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 issue as in rst-report-hub connector
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.
Is it because the variable CONNECTOR_UPDATE_EXISTING_DATA was deprecated? Should we remove it as now merging is done using confidence levels?
Proposed changes
Related issues
Checklist
Further comments