Skip to content

Conversation

@OlmeNav
Copy link
Contributor

@OlmeNav OlmeNav commented Sep 9, 2025

Context

Ensure that the CheckID, the class name and the file name where the check is defined all match. This corresponds to a TODO comment in the file prowler/lib/check/model.py, specifically in the __init__ method of the class Check.

Description

Add name consistency control at the end of the method. In case that names do not match, it is logged with 'logger.error'. There are some aspects to consider:

  • In some small tests I performed, if the CheckID is modified it does not break the execution itself, only the Check ID in the output is changed.
  • If the file name is modified, the corresponding .json file is not recognized and the execution aborts.
  • If the class name is modified, the scan completes but no findings remain. Therefore, it does not appear to be a critical problem.
  • The logger level at that point is 50, so the message is not actually registered. Maybe it should be logged with logger.critical, but I am not sure if it is that relevant.

Checklist

API

  • Verify if API specs need to be regenerated.
  • Check if version updates are required (e.g., specs, Poetry, etc.).
  • Ensure new entries are added to CHANGELOG.md, if applicable.

License

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

@OlmeNav OlmeNav requested review from a team as code owners September 9, 2025 15:43
@HugoPBrito HugoPBrito self-assigned this Sep 11, 2025
@codecov
Copy link

codecov bot commented Sep 11, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.90%. Comparing base (fbda66c) to head (3797ed5).
⚠️ Report is 39 commits behind head on master.

❗ There is a different number of reports uploaded between BASE (fbda66c) and HEAD (3797ed5). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (fbda66c) HEAD (3797ed5)
api 1 0
Additional details and impacted files
@@             Coverage Diff             @@
##           master    #8690       +/-   ##
===========================================
- Coverage   89.27%   75.90%   -13.38%     
===========================================
  Files         202       72      -130     
  Lines       18470     5039    -13431     
===========================================
- Hits        16490     3825    -12665     
+ Misses       1980     1214      -766     
Flag Coverage Δ
api ?
prowler 75.90% <100.00%> (+0.12%) ⬆️

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

Components Coverage Δ
prowler 75.90% <100.00%> (+0.12%) ⬆️
api ∅ <ø> (∅)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Member

@HugoPBrito HugoPBrito left a comment

Choose a reason for hiding this comment

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

Hi @OlmeNav,

Thanks for your contribution!
Could you please do the requested changes, merge the master branch and add an entry in the changelog under the Added section for v5.13?

Something like:

  • Equality validation for CheckID, filename and classname #8690

@cesararroba cesararroba added has-conflicts The PR has conflicts that needs to be resolved. and removed has-conflicts The PR has conflicts that needs to be resolved. labels Sep 12, 2025
@HugoPBrito HugoPBrito merged commit 035293b into prowler-cloud:master Sep 30, 2025
13 of 15 checks passed
vicferpoy pushed a commit that referenced this pull request Sep 30, 2025
…me in the Check class (#8690)

Co-authored-by: angelolmn <e.angelolm#go.ugr.es>
Co-authored-by: César Arroba <[email protected]>
Co-authored-by: Pepe Fagoaga <[email protected]>
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