prowler icon indicating copy to clipboard operation
prowler copied to clipboard

delete(extra7145): It is redundant to check_extra798

Open kagahd opened this issue 2 years ago • 4 comments

Context

check_extra7145 does the same as check_extra798 and can therefore be deleted

Description

Even the implementations of both checks are almost identical. The only differences are:

  1. slightly different wording of textFail, textPass and textInfo
  2. output format of lambda get-policy is text in extra798 but json in extra7145
  3. since the output formats are different, extra7145 uses additionally jq '. | fromjson'
  4. extra7142 tests explicitly on a ResourceNotFoundException while extra798 just tests on existence of a policy

License

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

kagahd avatar Oct 19 '22 15:10 kagahd

Hi @kagahd, good catch, thank you for your contribution!

But, why did you remove the cron from the refresh_aws_services_regions.yml?

sergargar avatar Oct 27 '22 09:10 sergargar

But, why did you remove the cron from the refresh_aws_services_regions.yml?

I removed it from my forked repo because the workflow run failed and alerted me daily by e-mail. However, I restored the file but to avoid the daily alerts I commented out the cron. I guess I shouldn't have done this, right? But how to avoid the failed runs respectively the daily alerts?

kagahd avatar Oct 27 '22 20:10 kagahd

I may have found out by myself why the workflow failed: it uses the branch prowler-3.0-dev which my forked repo hadn't had. To fix it, I just added that branch to my forked repo. Since then, the workflow is running without problems, so I activated the cron again as it's the case in the upstream prowler repo. Now you should be able to merge without conflicts.

kagahd avatar Oct 27 '22 21:10 kagahd

I see that the status is still awaiting response even though I answered your question already @sergargar. Do you need more input from me?

kagahd avatar Nov 02 '22 19:11 kagahd

Hi @kagahd, sorry for the delayed response. The check is duplicated and we included it in the roadmap of the new version 3 that we will release soon. Stay tuned for the release, thank you again for letting us know!

sergargar avatar Nov 08 '22 12:11 sergargar