-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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(output): Add HTML outputs to Prowler #4005
Conversation
…3986) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Sergio Garcia <38561120+sergargar@users.noreply.github.com>
You can check the documentation for this PR here -> SaaS Documentation |
You can check the documentation for this PR here -> SaaS Documentation |
Great job!! Please change the screenshot in the docs index and solve the kubernetes summary tables. |
You can check the documentation for this PR here -> SaaS Documentation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work bringing back the HTML format @pedrooot 👏
I left some comments since I don't understand some parts of the code. We need to make some parts clearer and fix the failing tests.
docs/tutorials/reporting.md
Outdated
@@ -294,14 +294,16 @@ The following code is an example output of the [JSON-ASFF](https://docs.aws.amaz | |||
???+ note | |||
Each finding is a `json` object within a list. | |||
|
|||
### HTML | |||
|
|||
The following photo is an example of the HTML output: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The following photo is an example of the HTML output: | |
The following image is an example of the HTML output: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
prowler/__main__.py
Outdated
if "html" in mode: | ||
add_html_footer( | ||
global_provider.output_options.output_filename, | ||
args.output_directory, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
args.output_directory, | |
global_provider.output_options.output_directory, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please modify this too in line 269.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
prowler/__main__.py
Outdated
fill_html_overview_statistics( | ||
stats, | ||
global_provider.output_options.output_filename, | ||
args.output_directory, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
args.output_directory, | |
global_provider.output_options.output_directory, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@@ -146,8 +146,8 @@ def __init_outputs_parser__(self): | |||
"-M", | |||
nargs="+", | |||
help="Output modes, by default csv and json-oscf are saved. When using AWS Security Hub integration, json-asff output is also saved.", | |||
default=["csv", "json-ocsf"], | |||
choices=["csv", "json-asff", "json-ocsf"], | |||
default=["csv", "json-ocsf", "html"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix the tests related with this change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
if not isinstance(finding, FindingOutput): | ||
check_id = finding.check_metadata.CheckID | ||
else: | ||
check_id = finding.check_id |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this change? The HTML should get this from the finding object.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True
prowler/lib/outputs/html/html.py
Outdated
timestamp, | ||
) | ||
from prowler.lib.logger import logger | ||
from prowler.lib.outputs.compliance.compliance import get_check_compliance |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shouldn't be needed here, please get that information from the finding object.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True
prowler/lib/outputs/html/html.py
Outdated
parameters = sys.argv[1:] | ||
for index, parameter in enumerate(parameters): | ||
if ( | ||
parameter == "--kubeconfig-file" | ||
and "/.kube/config" in parameters[index + 1] | ||
): | ||
parameters[index + 1] = "~/.kube/config" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this? Please leave this function jus to fill the header, if you need to do some other changes pass do them in another place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
# It is not pretty but useful | ||
# AWS_provider --> aws | ||
# GCP_provider --> gcp | ||
# Azure_provider --> azure |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to do it for K8s right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Forgot to update the comment but it's already done for Kubernetes
prowler/lib/outputs/outputs.py
Outdated
fill_html( | ||
file_descriptors["html"], finding_output, output_options | ||
) | ||
file_descriptors["html"].write("") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the report is written, first of all the html header is created and the method fill_html is called when creating the report.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know but I mean about file_descriptors["html"].write("")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fact, it's unneeded
You can check the documentation for this PR here -> SaaS Documentation |
1 similar comment
You can check the documentation for this PR here -> SaaS Documentation |
You can check the documentation for this PR here -> SaaS Documentation |
You can check the documentation for this PR here -> SaaS Documentation |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4005 +/- ##
==========================================
- Coverage 86.65% 86.23% -0.43%
==========================================
Files 777 777
Lines 24139 24292 +153
==========================================
+ Hits 20918 20947 +29
- Misses 3221 3345 +124 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work @pedrooot 👏 HTML is back 🚀
Description
HTML outputs are back for all the providers.
License
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.