Skip to content

Plugin healthecheck and update#1986

Merged
0ssigeno merged 33 commits intodevelopfrom
python_config_actions
Dec 1, 2023
Merged

Plugin healthecheck and update#1986
0ssigeno merged 33 commits intodevelopfrom
python_config_actions

Conversation

@0ssigeno
Copy link
Contributor

@0ssigeno 0ssigeno commented Nov 13, 2023

(Please add to the PR name the issue/s that this PR would close if merged by using a Github keyword. Example: <feature name>. Closes #999. If your PR is made by a single commit, please add that clause in the commit too. This is all required to automate the closure of related issues.)

Description

Please include a summary of the change and link to the related issue.

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue).
  • New feature (non-breaking change which adds functionality).
  • Breaking change (fix or feature that would cause existing functionality to not work as expected).

Checklist

  • I have read and understood the rules about how to Contribute to this project
  • The pull request is for the branch develop
  • A new plugin (analyzer, connector, visualizer, playbook, pivot or ingestor) was added or changed, in which case:
    • I strictly followed the documentation "How to create a Plugin"
    • Usage file was updated.
    • Advanced-Usage was updated (in case the plugin provides additional optional configuration).
    • If the plugin requires mocked testing, _monkeypatch() was used in its class to apply the necessary decorators.
    • I have dumped the configuration from Django Admin using the dumpplugin command and added it in the project as a data migration. [Doc]("How to create a Plugin")
    • If a File analyzer was added and it supports a mimetype which is not already supported, you added a sample of that type inside the archive test_files.zip and you added the default tests for that mimetype in test_classes.py.
    • If you created a new analyzer and it is free (does not require API keys), please add it in the FREE_TO_USE_ANALYZERS playbook in playbook_config.json.
    • Check if it could make sense to add that analyzer/connector to other freely available playbooks.
    • I have provided the resulting raw JSON of a finished analysis and a screenshot of the results.
  • If external libraries/packages with restrictive licenses were used, they were added in the Legal Notice section.
  • Linters (Black, Flake, Isort) gave 0 errors. If you have correctly installed pre-commit, it does these checks and adjustments on your behalf.
  • I have added tests for the feature/bug I solved (see tests folder). All the tests (new and old ones) gave 0 errors.
  • If changes were made to an existing model/serializer/view, the docs were updated and regenerated (check CONTRIBUTE.md).
  • If the GUI has been modified:
    • I have a provided a screenshot of the result in the PR.
    • I have created new frontend tests for the new component or updated existing ones.

Important Rules

  • If you miss to compile the Checklist properly, your PR won't be reviewed by the maintainers.
  • If your changes decrease the overall tests coverage (you will know after the Codecov CI job is done), you should add the required tests to fix the problem
  • Everytime you make changes to the PR and you think the work is done, you should explicitly ask for a review. After being reviewed and received a "change request", you should explicitly ask for a review again once you have made the requested changes.

@0ssigeno
Actions
44175e9 
Signed-off-by: 0ssigeno <[email protected]>
code-review-doctor[bot]
code-review-doctor bot suggested changes Nov 13, 2023
View reviewed changes
Copy link
Contributor

@code-review-doctor code-review-doctor bot left a comment

Choose a reason for hiding this comment

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

Looks good. Worth considering though. View full project report here.

@0ssigeno
Blake
9b5905f 
Signed-off-by: 0ssigeno <[email protected]>
@0ssigeno
Blake
42a43c4 
Signed-off-by: 0ssigeno <[email protected]>
code-review-doctor[bot]
code-review-doctor bot suggested changes Nov 13, 2023
View reviewed changes
Copy link
Contributor

@code-review-doctor code-review-doctor bot left a comment

Choose a reason for hiding this comment

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

Worth considering. View full project report here.

github-advanced-security[bot]
github-advanced-security bot found potential problems Nov 13, 2023
View reviewed changes
api_app/views.py
raise ValidationError({"detail": "No healthcheck implemented"})
health_status = python_obj.health_check(request.user)
except NotImplementedError as e:
raise ValidationError({"detail": str(e)})

Check warning

Code scanning / CodeQL

Information exposure through an exception

[Stack trace information](1) flows to this location and may be exposed to an external user.
api_app/views.py
try:
update_status = python_obj.update()
except NotImplementedError as e:
raise ValidationError({"detail": str(e)})

Check warning

Code scanning / CodeQL

Information exposure through an exception

[Stack trace information](1) flows to this location and may be exposed to an external user.
@0ssigeno
Fix tests
552b0dd 
Signed-off-by: 0ssigeno <[email protected]>
@0ssigeno
Fixes
4368e94 
Signed-off-by: 0ssigeno <[email protected]>
code-review-doctor[bot]
code-review-doctor bot suggested changes Nov 13, 2023
View reviewed changes
Copy link
Contributor

@code-review-doctor code-review-doctor bot left a comment

Choose a reason for hiding this comment

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

Worth considering. View full project report here.

0ssigeno and others added 7 commits November 13, 2023 17:33
@0ssigeno @code-review-doctor
Update api_app/classes.py
16f3d98 
Co-authored-by: code-review-doctor[bot] <72320148+code-review-doctor[bot]@users.noreply.github.com>
@0ssigeno
Fixes
e7a1aec 
Signed-off-by: 0ssigeno <[email protected]>
@0ssigeno
Fixes
a3fa0f2 
Signed-off-by: 0ssigeno <[email protected]>
@0ssigeno
Fixes
5c60357 
Signed-off-by: 0ssigeno <[email protected]>
@0ssigeno
Fixes
86605e2 
Signed-off-by: 0ssigeno <[email protected]>
@0ssigeno
Fixes
c5ed961 
Signed-off-by: 0ssigeno <[email protected]>
@0ssigeno
Fixes
285eb2c 
Signed-off-by: 0ssigeno <[email protected]>
@codecov
Copy link

codecov bot commented Nov 14, 2023 •
edited
Loading

Codecov Report

Merging #1986 (c694419) into develop (aa8820f) will increase coverage by 8.83%.
Report is 1528 commits behind head on develop.
The diff coverage is 75.47%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1986      +/-   ##
===========================================
+ Coverage    66.75%   75.59%   +8.83%     
===========================================
  Files           95      458     +363     
  Lines         3706    13763   +10057     
  Branches       519     1431     +912     
===========================================
+ Hits          2474    10404    +7930     
- Misses         941     2798    +1857     
- Partials       291      561     +270
Files Coverage Δ
api_app/analyzers_manager/admin.py 100.00% <100.00%> (ø)
api_app/analyzers_manager/apps.py 100.00% <100.00%> (ø)
api_app/analyzers_manager/constants.py 100.00% <100.00%> (ø)
api_app/analyzers_manager/exceptions.py 100.00% <100.00%> (ø)
...app/analyzers_manager/file_analyzers/boxjs_scan.py 100.00% <100.00%> (ø)
...pi_app/analyzers_manager/file_analyzers/onenote.py 100.00% <100.00%> (ø)
...pi_app/analyzers_manager/file_analyzers/peframe.py 82.35% <100.00%> (ø)
...i_app/analyzers_manager/file_analyzers/rtf_info.py 48.64% <100.00%> (ø)
...s_manager/file_analyzers/xlm_macro_deobfuscator.py 62.50% <100.00%> (ø)
api_app/analyzers_manager/filters.py 100.00% <100.00%> (ø)
... and 235 more

... and 269 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ecfef21...c694419. Read the comment docs.

@0ssigeno 0ssigeno requested a review from mlodic November 14, 2023 14:48
mlodic
mlodic reviewed Nov 14, 2023
View reviewed changes
Copy link
Member

@mlodic mlodic left a comment

Choose a reason for hiding this comment

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

could you please add some doc about that feature and how to add a new custom healthcheck/update for a plugin? ❤️

@0ssigeno
Refactor
12984a1 
Signed-off-by: 0ssigeno <[email protected]>
code-review-doctor[bot]
code-review-doctor bot suggested changes Nov 15, 2023
View reviewed changes
Copy link
Contributor

@code-review-doctor code-review-doctor bot left a comment

Choose a reason for hiding this comment

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

Some food for thought. View full project report here.

@0ssigeno
django doctor
90473df 
Signed-off-by: 0ssigeno <[email protected]>
code-review-doctor[bot]
code-review-doctor bot suggested changes Nov 15, 2023
View reviewed changes
Copy link
Contributor

@code-review-doctor code-review-doctor bot left a comment

Choose a reason for hiding this comment

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

Looks good. Worth considering though. View full project report here.

0ssigeno and others added 4 commits November 15, 2023 10:52
@0ssigeno
Merge branch 'develop' into python_config_actions
572da8c
@0ssigeno
Fixes
317ac8a 
Signed-off-by: 0ssigeno <[email protected]>
@0ssigeno
Added documentation
92edf08 
Signed-off-by: 0ssigeno <[email protected]>
@0ssigeno @code-review-doctor
Update api_app/migrations/0050_python_module_update_task.py
0661c9d 
Co-authored-by: code-review-doctor[bot] <72320148+code-review-doctor[bot]@users.noreply.github.com>
@0ssigeno
Blake
109d12c 
Signed-off-by: 0ssigeno <[email protected]>
@0ssigeno 0ssigeno requested a review from mlodic November 15, 2023 11:14
@0ssigeno
Performance
7036c55 
Signed-off-by: 0ssigeno <[email protected]>
@0ssigeno
Added migration files
58884f6 
Signed-off-by: 0ssigeno <[email protected]>
@0ssigeno
Fix condition
abd5451 
Signed-off-by: 0ssigeno <[email protected]>
@0ssigeno
Fix contains
1317fc0 
Signed-off-by: 0ssigeno <[email protected]>
@0ssigeno
Fixes
372be32 
Signed-off-by: 0ssigeno <[email protected]>
@0ssigeno
Fixes
0c8ebed 
Signed-off-by: 0ssigeno <[email protected]>
@0ssigeno
Typo
3a8c1b8 
Signed-off-by: 0ssigeno <[email protected]>
@0ssigeno
Fix
c442f4d 
Signed-off-by: 0ssigeno <[email protected]>
@0ssigeno
Fix
0c56281 
Signed-off-by: 0ssigeno <[email protected]>
@mlodic
Copy link
Member

mlodic commented Nov 20, 2023

I'll kindly ask you to verify that the content of this paragraph in the doc is still valid and to update it in case it is needed

@0ssigeno
Update docs
2505e48 
Signed-off-by: 0ssigeno <[email protected]>
@0ssigeno 0ssigeno merged commit ecaacd2 into develop Dec 1, 2023
@0ssigeno 0ssigeno deleted the python_config_actions branch December 1, 2023 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mlodic mlodic mlodic approved these changes

+1 more reviewer

@code-review-doctor code-review-doctor[bot] code-review-doctor[bot] requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@0ssigeno @mlodic