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

[Group IB] Update Group-IB connector #3411

Merged
merged 8 commits into from
Feb 20, 2025

Conversation

Kchekh
Copy link
Contributor

@Kchekh Kchekh commented Feb 13, 2025

Proposed changes

Group-IB integration documentation update
Updating docker-compose and integration configuration

Related issues

#3405

Checklist

  • I consider the submitted work as finished
  • I tested the code for its functionality using different use cases
  • I added/update the relevant documentation (either on github or on notion)
  • Where necessary I refactored code to improve the overall quality

Further comments

--

@romain-filigran romain-filigran added this to the PRs backlog milestone Feb 13, 2025
@helene-nguyen helene-nguyen self-assigned this Feb 13, 2025
@SamuelHassine SamuelHassine force-pushed the master branch 2 times, most recently from b513b96 to 11a05bf Compare February 14, 2025 12:49
@helene-nguyen helene-nguyen changed the title Update Group-IB connector [Group IB] Update Group-IB connector Feb 14, 2025
@@ -63,23 +62,81 @@ def __init__(self):

def _load_config(self) -> dict:
"""
Load the configuration from the YAML file
:return: Configuration dictionary
Loads the configuration from `config.yml` or falls back to `config.yml.sample` if `config.yml` does not exist.
Copy link
Contributor

Choose a reason for hiding this comment

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

   Loads the configuration from `config.yml` or falls back to `config.yml.sample` if `config.yml` does not exist.

Thanks for your contribution! However, the changes related to the config.yml.sample fallback are not aligned with the project's convention. In this repository, we maintain consistency by only falling back to an empty dictionary ({}) when config.yml is missing. Making an exception for this case would introduce inconsistency across the project.

Would you mind adjusting the implementation to stick to this approach?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello @flavienSindou ,
Thanks for your review and your note about the configuration. Can you please tell me, do you mean to completely remove reading config.yml.sample from the project?

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for your answer,

I meant the config.yml.sample file should be an example file about how to create a config.yaml file and should not be used into the code.

Comment on lines 81 to 86
for file_path in [config_file_path, sample_config_file_path]:
if file_path.is_file():
with open(file_path, "r", encoding="utf-8") as file:
return yaml.load(file, Loader=yaml.FullLoader)

return {}
Copy link
Contributor

Choose a reason for hiding this comment

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

See comment above

@Kchekh
Copy link
Contributor Author

Kchekh commented Feb 16, 2025

Hello @flavienSindou ,
I made the change to read the configuration. At the moment only values from the environment or from config.yml are read. I also want to note that because of the large number of environment settings and in config.yml to write values directly into the connector seems to me not a suitable solution, because the code will be very much stretched. That's why I'm trying to implement an approach where the settings for integration will be identical both in .env and in config.yml and based on what the client uses the keys will be created for the appropriate variant

@flavienSindou flavienSindou merged commit f00b38b into OpenCTI-Platform:master Feb 20, 2025
4 checks passed
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.

[GroupIB] doc: unaligned config var names between documentation and the code
4 participants