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

fix(custom): execute custom checks #4202

Merged
merged 3 commits into from
Jun 7, 2024

Conversation

sejimhp
Copy link
Contributor

@sejimhp sejimhp commented Jun 7, 2024

Context

Custom rules were not available and I would like to to fix it.

It is caused by copying the custom checks folder after loading the checks.
so, added when files are copied

Description

copying the custom check in the parse_checks_from_folder function here
But check loading is called in load_checks_to_execute before that

There was no process to assign a custom check for that transition, so the custom check was not executed!

Added support for updating pre-loaded sets at the time of copying custom check folders.

Check

Here is an example of gcp.
Prepare the following directory structure rules.

custom_rule
└── cloudsql_test_custom_rule
    ├── __init__.py
    ├── cloudsql_test_custom_rule.metadata.json
    └── cloudsql_test_custom_rule.py

Execute the following command

without custome rule scan

prowler gcp --project-ids {project_id}

with custome rule scan

prowler gcp --project-ids {project_id} -x custom_rule

Confirmed that the number of scans performed and the number of compliant evaluations changes

License

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@sejimhp sejimhp requested review from a team as code owners June 7, 2024 06:33
@pedrooot
Copy link
Member

pedrooot commented Jun 7, 2024

Hey! @sejimhp This looks great! Could you fix the tests in order to ensure that all it's correct? Thank you 💯

@pedrooot pedrooot self-assigned this Jun 7, 2024
@sejimhp
Copy link
Contributor Author

sejimhp commented Jun 7, 2024

@pedrooot Thank you for review!
I fixed the test.

@sejimhp
Copy link
Contributor Author

sejimhp commented Jun 7, 2024

I had to add an additional push because it failed in the format.

Copy link

codecov bot commented Jun 7, 2024

Codecov Report

Attention: Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.

Project coverage is 86.67%. Comparing base (0a41ec4) to head (2d0ca66).
Report is 1 commits behind head on master.

Files Patch % Lines
prowler/__main__.py 0.00% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4202   +/-   ##
=======================================
  Coverage   86.66%   86.67%           
=======================================
  Files         818      818           
  Lines       25674    25675    +1     
=======================================
+ Hits        22251    22253    +2     
+ Misses       3423     3422    -1     

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

@sejimhp
Copy link
Contributor Author

sejimhp commented Jun 7, 2024

The coverage drop seems to be caused by main.py, but I couldn't find a test for it...

@sergargar sergargar changed the title bugfix: fix custom rules not executing fix(custom): execute custom checks Jun 7, 2024
Copy link
Member

@sergargar sergargar left a comment

Choose a reason for hiding this comment

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

Good catch @sejimhp, thank you so much for the fix!

@sergargar sergargar merged commit 2a139e3 into prowler-cloud:master Jun 7, 2024
10 of 11 checks passed
sergargar pushed a commit that referenced this pull request Jun 7, 2024
sergargar pushed a commit that referenced this pull request Jun 7, 2024
jfagoagas added a commit that referenced this pull request Jun 17, 2024
@@ -180,7 +180,8 @@ def prowler():

# Import custom checks from folder
if checks_folder:
parse_checks_from_folder(global_provider, checks_folder)
custom_checks = parse_checks_from_folder(global_provider, checks_folder)
checks_to_execute.update(custom_checks)
Copy link
Contributor

Choose a reason for hiding this comment

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

No, this is wrong and a regression! Including checks from the checks_folder does not necessarily mean that we want to execute all of them! You may want to execute only checks that are defined by parameter -c or --checks. See prowlers documentation. Your code prevents this behavior. It worked still in the previous prowler version 4.2.3 but it's broken now in the actual v4.2.4.

Copy link
Member

Choose a reason for hiding this comment

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

This workaround fixed that #4256

Copy link
Contributor

Choose a reason for hiding this comment

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

This workaround fixed that #4256

That's great!
PS: I clicked on "submit review" only the next day of my comment. That's why my comment only appeared now, after having fixed the issue already.

Copy link
Member

Choose a reason for hiding this comment

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

No problem, thanks!

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.

None yet

5 participants