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

feat: add Integration model #109

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

feat: add Integration model #109

wants to merge 16 commits into from

Conversation

bogdanoniga
Copy link

@bogdanoniga bogdanoniga commented Apr 10, 2023

Integration model is meant to be a base model for service integrations (e.g. AWS, GCP, etc.). It serves two main functions:

  • Manage the service integrations and related credentials within the UI;
  • Manage the actions/commands to be performed per service integration;
  • Map assets to service integrations in a similar way as Source model does.

Each service integration has its own particularities and actions, therefore, this model will be inherited and extended per service integration's needs.

@bogdanoniga bogdanoniga requested a review from a team as a code owner April 10, 2023 14:48
@DDuarte
Copy link
Contributor

DDuarte commented Apr 10, 2023

I like the idea but not the implementation, in particular the hardcoded values in IntegrationType - it doesn't allow for composability.

IMO it would be better if each "integration" app registered itself on init, similar to how the django apps already work. It could even be an extended version of AppConfig with metadata relevant for integrations.

@bogdanoniga
Copy link
Author

bogdanoniga commented Apr 10, 2023

I like the idea but not the implementation, in particular the hardcoded values in IntegrationType - it doesn't allow for composability.

IMO it would be better if each "integration" app registered itself on init, similar to how the django apps already work. It could even be an extended version of AppConfig with metadata relevant for integrations.

IntegrationType is not an important field, we can rely on the child name. I'll remove the field

@bogdanoniga
Copy link
Author

bogdanoniga commented Apr 10, 2023

I was thinking to have an extended Integration model part of new django applications (if required) and match the application particularities with an extended list of fields. For instance, a new module named django-github, meant to sync data from Github, will extend the Integration model to match Github particularities and actions. Therefore, the credentials will be different for Github compared to other services (e.g. AWS) which requires new fields to be added

@bogdanoniga
Copy link
Author

I like the idea but not the implementation, in particular the hardcoded values in IntegrationType - it doesn't allow for composability.

IMO it would be better if each "integration" app registered itself on init, similar to how the django apps already work. It could even be an extended version of AppConfig with metadata relevant for integrations.

With this proposal I would like to dynamically manage the integrations at application level at any time after deployment not as an integration config at init phase. I'm trying to move away from the current approach which defines integration credentials as env vars to a more dynamic way of creating and managing integrations via UI

@bogdanoniga bogdanoniga changed the title feat: add Integration module feat: add Integration model Apr 10, 2023
@bogdanoniga bogdanoniga added the enhancement New feature or request label Apr 10, 2023
@bogdanoniga
Copy link
Author

As an example, https://github.com/surface-security/django-github uses Integration model as part of the testapp

@bogdanoniga
Copy link
Author

somebody has a bit of free time to review this PR?

surface/inventory/models.py Outdated Show resolved Hide resolved
surface/inventory/models.py Outdated Show resolved Hide resolved
bogdanoniga and others added 3 commits May 8, 2023 23:29
Co-authored-by: Gustavo Silva <gustavosantaremsilva@gmail.com>
Signed-off-by: Bogdan Oniga <bogdan@oniga.me>
Co-authored-by: Gustavo Silva <gustavosantaremsilva@gmail.com>
Signed-off-by: Bogdan Oniga <bogdan@oniga.me>
Co-authored-by: Gustavo Silva <gustavosantaremsilva@gmail.com>
Signed-off-by: Bogdan Oniga <bogdan@oniga.me>
@bogdanoniga bogdanoniga requested a review from gsilvapt May 8, 2023 20:31
@gsilvapt gsilvapt added this to the v1.2.0 milestone May 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants