add wafv2 modules #450
add wafv2 modules #450
Conversation
ansibullbot
added
community_review
integration
|
aws terminator needs rw permissions for wafv2
|
|
Required: ansible/ansible#73975 for pass 2.9 integration tests |
ansibullbot
added
needs_rebase
|
Ok, shippable is running to many tests in parallel. Here the wafv2 test pass I guess someone must kick one single failed test, until all pass. |
ansibullbot
added
community_review
and removed
needs_revision
There was a problem hiding this comment.
Thanks for your patience @markuman (and all the new modules!). If possible I'd like to get a second review on this, maybe @tremble or @goneri? Just a few docs nits and test comments left from me.
I'm planning to release 1.5 in the next 1-2 weeks, and I'm happy block that on this one as we're really close.
| return response | ||
|
|
||
|
|
||
| def wafv2_snake_dict_to_camel_dict(a): |
There was a problem hiding this comment.
May I suggest to add a unit-test for this function?
There was a problem hiding this comment.
hm basically it is already covered by the integration test.
I've added different rules circumstances that must be covered by those functions and that must not be covered by those functions.
nested_byte_values_to_stringsandbyte_values_to_strings_before_comparebyte_match_statement: https://github.com/ansible-collections/community.aws/pull/450/files#diff-766b578e1e76effb51404af767b665a1dbd0a00d378ac1645cbdbeb1916b006fR21or_statement: https://github.com/ansible-collections/community.aws/pull/450/files#diff-766b578e1e76effb51404af767b665a1dbd0a00d378ac1645cbdbeb1916b006fR237- a statement which will not be affected by the functions: https://github.com/ansible-collections/community.aws/pull/450/files#diff-17703aae6879ed6c730c426f245c29a60a0fad10c392b23ad582e1f7ddc01cd1R49
wafv2_snake_dict_to_camel_dict()- https://github.com/ansible-collections/community.aws/pull/450/files#diff-77dad05e606a9ef34f8fabbfee7a7803668278569c1e1d14360705f640966d8aR125
ip_set_reference_statementbecomesIPSetReferenceStatement.
There was a problem hiding this comment.
Yes I know and this is the reason why I put suggest in bold. PLEASE, Don't force yourself to do it!
I see two reasons to write unit-tests in your case:
- sometime the behavior of the API change, it's a way to have a copy of the original expect input.
- unit-tests run way way way way faster than integration tests. And if they fail we can configure the CI to avoid the functional tests. This way we save the valuable developer time and a lot of CI resources.
- it's also rather easy for a developer to be able to run the unit-tests locally. It's a different story for the functional tests.
Well, that's 3 reasons actually ^^.
There was a problem hiding this comment.
For what it's worth you should be able to copy most of the unit-test framework for these transforms from https://github.com/ansible-collections/amazon.aws/blob/main/tests/unit/module_utils/core/test_scrub_none_parameters.py
| retval[item] = a.get(item) | ||
| return retval | ||
|
|
||
|
|
There was a problem hiding this comment.
Well, unit-test would be great for the others too :-).
add wafv2 modules Reviewed-by: https://github.com/apps/ansible-zuul This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections@2ee7d6f
add wafv2 modules Reviewed-by: https://github.com/apps/ansible-zuul This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections@2ee7d6f
add wafv2 modules Reviewed-by: https://github.com/apps/ansible-zuul This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections@60d7c68
add wafv2 modules Reviewed-by: https://github.com/apps/ansible-zuul
add wafv2 modules Reviewed-by: https://github.com/apps/ansible-zuul
add wafv2 modules Reviewed-by: https://github.com/apps/ansible-zuul
…nsible-collections#450) Add botocore requirements to s3_bucket ownership control management SUMMARY (get|set|delete)_bucket_ownership_controls requires botocore >= 1.18.11 Because we state our minimum supported version of botocore is 1.16.0 we need to explicitly call this requirement for management of bucket ownership controls. ISSUE TYPE Bugfix Pull Request COMPONENT NAME s3_bucket ADDITIONAL INFORMATION fixes: ansible-collections#449 Reviewed-by: Alina Buzachis <None> Reviewed-by: None <None>
…nsible-collections#404) Add constraints.txt and requirements.txt for unit/integration tests SUMMARY Now that we state that we support specific minimum versions of the AWS SDKs, make sure we base our unit/integration tests against them such that modules need to explicitly test/request newer versions of the SDKs. ISSUE TYPE Feature Pull Request COMPONENT NAME tests/integration tests/unit ADDITIONAL INFORMATION Once merged into amazon.aws we should merge this into community.aws Depends-On: ansible-collections#453 Depends-On: ansible-collections#454 Depends-On: ansible-collections#450 Depends-On: ansible-collections#496 See also: ansible/ansible-zuul-jobs#991 Reviewed-by: Jill R <None> Reviewed-by: None <None>
SUMMARY
Add wafv2 modules
ISSUE TYPE
COMPONENT NAME
ADDITIONAL INFORMATION
The modules share same
module_utils/wafv2.pycodebase.Furthermore, the integration test needs an Application Loadbalancer, that needs itself needs a target group, a subnet and a VPC. This is the reason why the integration test is that long.