Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix(wafv2): only list resources for regional Web ACLs #5811

Merged
merged 1 commit into from
Nov 18, 2024

Conversation

HugoPBrito
Copy link
Member

@HugoPBrito HugoPBrito commented Nov 18, 2024

Context

An WAFInvalidParameterException error message was appearing while listing resources due to global ACLs being included in the call, which isn't supported by boto3.

Fix #5805

Description

Logic behind this error has been corrected and _list_resources_for_web_acl only lists regional ACLs.

Checklist

License

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

@HugoPBrito HugoPBrito requested review from a team as code owners November 18, 2024 15:22
@github-actions github-actions bot added the provider/aws Issues/PRs related with the AWS provider label Nov 18, 2024
Copy link
Member

@jfagoagas jfagoagas left a comment

Choose a reason for hiding this comment

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

Why not to catch the exception?

@MrCloudSec
Copy link
Member

MrCloudSec commented Nov 18, 2024

Why not to catch the exception?

This exception won't appear again with this change since we were using this call for Global Web ACLs and only works for regional ones.

@MrCloudSec MrCloudSec changed the title fix(wafv2): WAFInvalidParameterException fix(wafv2): only list resources for regional Web ACLs Nov 18, 2024
@MrCloudSec MrCloudSec requested a review from jfagoagas November 18, 2024 15:27
@HugoPBrito
Copy link
Member Author

Why not to catch the exception?

AWS does not support _list_resources_for_web_acl in global ACLs, because that listing has to be done in Cloudfront side:
https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/wafv2/client/list_resources_for_web_acl.html

I don't see the point of catching it if we can avoid the failing scenario instead.

@MrCloudSec MrCloudSec self-requested a review November 18, 2024 15:30
@jfagoagas
Copy link
Member

Thank you for the clarification @HugoPBrito @MrCloudSec ❤️

@HugoPBrito
Copy link
Member Author

Thank you for the clarification @HugoPBrito @MrCloudSec ❤️

🫶

Copy link

codecov bot commented Nov 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.87%. Comparing base (a25a614) to head (98bb17e).
Report is 705 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #5811   +/-   ##
=======================================
  Coverage   89.87%   89.87%           
=======================================
  Files        1132     1132           
  Lines       35275    35275           
=======================================
+ Hits        31703    31705    +2     
+ Misses       3572     3570    -2     

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

@MrCloudSec MrCloudSec merged commit 572d5a1 into master Nov 18, 2024
12 checks passed
@MrCloudSec MrCloudSec deleted the PRWLR-5398-fix-issue-5805 branch November 18, 2024 16:09
@prowler-bot prowler-bot added the was-backported The PR was successfully backported to the target branch label Nov 18, 2024
@prowler-bot
Copy link
Collaborator

💚 All backports created successfully

Status Branch Result
v4.5

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

@jfagoagas jfagoagas added the backport-to-v4.5 Backport PR to the v4.5 branch label Dec 5, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
backport-to-v4.5 Backport PR to the v4.5 branch provider/aws Issues/PRs related with the AWS provider was-backported The PR was successfully backported to the target branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Invalid WAF arn
4 participants