-
Notifications
You must be signed in to change notification settings - Fork 434
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
[Spycloud] Create external import connector #3347
base: master
Are you sure you want to change the base?
Conversation
2c7d44e
to
abf331c
Compare
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.
Nice and clear documented code.
Some small remarks but a really good implementation.
You should add a test-requirements.txt file for the CI to launch your tests
I test the connector locally and I'll approve it.
```mermaid | ||
flowchart LR | ||
A[Spycloud] -->|get data periodically| B(Connector) | ||
B --> C{Process breach records} | ||
C -->|convert to| D1[STIX bundle] | ||
C -->|convert to| D2[STIX bundle] | ||
C -->|convert to| D3[STIX bundle] | ||
D1 & D2 & D3 -->|send to| E(OpenCTI) | ||
``` |
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 think this is considered as general documentation of the connector scheduler. I would replace it with a link to the dedicated page https://filigran.io/auto-backpressue-control-octi-connectors/#h-purpose-of-the-scheduler
from spycloud_connector.models.opencti import Author, Incident, TLPMarking | ||
|
||
|
||
def mock_valid_author(): |
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.
nitpicking ;)
def mock_valid_author(): | |
def fake_valid_author(): |
return Author(name="Valid Author", identity_class="organization") | ||
|
||
|
||
def mock_valid_markings(): |
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.
nitpicking ;)
def mock_valid_markings(): | |
def fake_valid_markings(): |
) | ||
|
||
|
||
def mock_valid_author(): |
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.
def mock_valid_author(): | |
def fake_valid_author(): |
return Author(name="Valid Author", identity_class="organization") | ||
|
||
|
||
def mock_valid_markings(): |
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.
def mock_valid_markings(): | |
def fake_valid_markings(): |
# When: we try to create a DomainName instance | ||
# Then: a ValidationError should be raised | ||
with pytest.raises(ValidationError) as err: | ||
DomainName(**input_data_dict) |
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.
nitpicking
DomainName(**input_data_dict) | |
DomainName.model_validate(**input_data_dict) |
Usually a model called with constructor should fill all kwargs explicitly, when using de-serialization or structures you would rather use a dedicated factory.
data = mock_spycloud_client._request(method="GET", url=mock_request["url"]) | ||
|
||
# Then None should be returned | ||
assert data is None |
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.
URL does not seem to be parametrized here and could be remove
{ | ||
"url": f"{TEST_API_BASE_URL}/breach/catalog/:breach_catalog_id", | ||
"status_code": 200, | ||
"response_body": get_data_sample("breach_catalog_api_response.json"), |
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.
MUST be deleted before merge into master
+ re-organize models directory
0ae8871
to
484f99c
Compare
Proposed changes
Related issues
Checklist