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(glimps): use py-gdetect client + add get status and wait for result actions #1169

Merged
merged 12 commits into from
Jan 21, 2025

Conversation

glimps-cbi
Copy link
Contributor

…ult actions

@glimps-cbi glimps-cbi force-pushed the feat/add_glimps_actions branch 2 times, most recently from 033341e to 9be9c26 Compare November 15, 2024 13:48
@glimps-glv glimps-glv force-pushed the feat/add_glimps_actions branch 2 times, most recently from 25d44f3 to a704637 Compare November 19, 2024 07:46
@glimps-cbi glimps-cbi force-pushed the feat/add_glimps_actions branch from a704637 to 0adeecf Compare November 19, 2024 08:00
@jeromefellus-sekoia jeromefellus-sekoia changed the title feat(glimps): use py-gdetect client + add get status and wait for res… feat(glimps): use py-gdetect client + add get status and wait for result actions Nov 19, 2024
Copy link
Contributor

@jeromefellus-sekoia jeromefellus-sekoia left a comment

Choose a reason for hiding this comment

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

Thank you for your contrib ! 🎉

@glimps-glv glimps-glv force-pushed the feat/add_glimps_actions branch from 0adeecf to 5894cdd Compare November 19, 2024 16:28
Copy link

github-actions bot commented Nov 19, 2024

Test Results

32 tests   - 72   24 ✅  - 79   1s ⏱️ -1s
 1 suites ± 0    8 💤 + 7 
 1 files   ± 0    0 ❌ ± 0 

Results for commit cda1034. ± Comparison against base commit 7ba5509.

This pull request removes 104 and adds 32 tests. Note that renamed tests count towards both.
tests.ic_oc_triggers.test_alerts ‑ test_alert_trigger_filter_by_rule
tests.ic_oc_triggers.test_alerts ‑ test_comment_trigger_filter_by_rule
tests.ic_oc_triggers.test_alerts ‑ test_comment_trigger_filter_notification_function
tests.ic_oc_triggers.test_alerts ‑ test_invalid_events_dont_triggers_comments_added
tests.ic_oc_triggers.test_alerts ‑ test_securityalertstrigger_handle_alert_invalid_message
tests.ic_oc_triggers.test_alerts ‑ test_securityalertstrigger_handle_alert_send_message
tests.ic_oc_triggers.test_alerts ‑ test_securityalertstrigger_handler_dispatch_alert_message
tests.ic_oc_triggers.test_alerts ‑ test_securityalertstrigger_init
tests.ic_oc_triggers.test_alerts ‑ test_securityalertstrigger_retrieve_alert_from_api
tests.ic_oc_triggers.test_alerts ‑ test_securityalertstrigger_retrieve_alert_from_api_exp_raised
…
tests.test_deprecated ‑ test_integration_retrieve_analysis
tests.test_deprecated ‑ test_retrieve_analysis
tests.test_deprecated ‑ test_submit_file_to_be_analysed
tests.test_deprecated ‑ test_submit_file_to_be_analysed_filesize_exceed
tests.test_deprecated ‑ test_submit_file_to_be_analysed_succeed
tests.test_export ‑ test_export_bad_format
tests.test_export ‑ test_export_bad_layout
tests.test_export ‑ test_export_error
tests.test_export ‑ test_export_succeed
tests.test_export ‑ test_integration_get_export
…
This pull request removes 1 skipped test and adds 8 skipped tests. Note that renamed tests count towards both.
tests.ic_oc_triggers.test_intelligence ‑ test_run
tests.test_deprecated ‑ test_integration_retrieve_analysis
tests.test_deprecated ‑ test_submit_file_to_be_analysed
tests.test_export ‑ test_integration_get_export
tests.test_get_status ‑ test_integration_get_status
tests.test_retrieve_analysis ‑ test_integration_retrieve_analysis
tests.test_search_analysis_by_sha256 ‑ test_integration_search_analysis
tests.test_submit_file_to_be_analysed ‑ test_integration_submit_file_to_be_analysed
tests.test_submit_file_to_be_analysed ‑ test_integration_wait_for_analysis

♻️ This comment has been updated with latest results.

Copy link

codecov bot commented Nov 19, 2024

Codecov Report

Attention: Patch coverage is 97.35683% with 6 lines in your changes missing coverage. Please review.

Project coverage is 90.58%. Comparing base (7ba5509) to head (cda1034).
Report is 13 commits behind head on main.

Files with missing lines Patch % Lines
Glimps/glimps/deprecated.py 87.75% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1169      +/-   ##
==========================================
+ Coverage   90.03%   90.58%   +0.55%     
==========================================
  Files          61       69       +8     
  Lines        2790     3017     +227     
  Branches      103      103              
==========================================
+ Hits         2512     2733     +221     
- Misses        240      246       +6     
  Partials       38       38              
Flag Coverage Δ
Glimps 97.35% <97.35%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@squioc squioc left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution, and sorry for the delai of this review.
In order to not break existing automations, we have to keep the old actions.

Can you restore the declaration of the old actions and prefix their name with
[DEPRECATED]? Same for the python code, can you restore it and maybe move it in a deprecated.py file? Thank you.

Glimps/manifest.json Show resolved Hide resolved
Glimps/tests/conftest.py Outdated Show resolved Hide resolved
Glimps/tests/test_export.py Outdated Show resolved Hide resolved
Glimps/tests/test_get_status.py Outdated Show resolved Hide resolved
Glimps/action_retrieve_the_analysis.json Outdated Show resolved Hide resolved
@glimps-glv glimps-glv force-pushed the feat/add_glimps_actions branch 3 times, most recently from c1c1daa to aed719a Compare December 16, 2024 08:24
@glimps-cbi glimps-cbi force-pushed the feat/add_glimps_actions branch from aed719a to 862cf51 Compare December 16, 2024 08:38
@glimps-glv glimps-glv force-pushed the feat/add_glimps_actions branch from 862cf51 to 4901e72 Compare December 18, 2024 09:19
@glimps-glv
Copy link
Contributor

@squioc @jeromefellus-sekoia @glimps-cbi plusieurs modifs faites :

Copy link
Collaborator

@squioc squioc left a comment

Choose a reason for hiding this comment

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

Thank you for the changes.
I detected an issue in main.py.
Except this, the code looks good to me.

Glimps/main.py Outdated Show resolved Hide resolved
squioc
squioc previously requested changes Dec 20, 2024
Glimps/main.py Outdated Show resolved Hide resolved
@glimps-glv glimps-glv force-pushed the feat/add_glimps_actions branch from eab4229 to 3c779e5 Compare December 20, 2024 08:26
@glimps-glv glimps-glv force-pushed the feat/add_glimps_actions branch from 3c779e5 to 758875e Compare January 21, 2025 13:38
@jeromefellus-sekoia
Copy link
Contributor

@squioc I've tested the actions on our pre-prod platform via custom integration. Everything seems ok, all your comments have been applied.

Ready for merge ?

@jeromefellus-sekoia
Copy link
Contributor

oops hold on ! I pushed the wrong commit where I tampered the actions UUID to allow testing without shadowing on the pre-prod. Let me push the right commits, sorry about it

auto-merge was automatically disabled January 21, 2025 15:49

Head branch was pushed to by a user without write access

@glimps-glv glimps-glv force-pushed the feat/add_glimps_actions branch from cb91e90 to cda1034 Compare January 21, 2025 16:02
@jeromefellus-sekoia
Copy link
Contributor

Thanks @glimps-glv all good now

@jeromefellus-sekoia jeromefellus-sekoia merged commit 20bc3c3 into SEKOIA-IO:main Jan 21, 2025
181 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.

4 participants