Skip to content
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.

Sign up for GitHub

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

Jump to bottom

add wafv2_ip_set module #449

Merged
merged 9 commits into from
Apr 8, 2021
Merged

add wafv2_ip_set module #449

merged 9 commits into from
Apr 8, 2021

Conversation

markuman
Copy link
Member

@markuman markuman commented Mar 1, 2021

SUMMARY

Add new module to handle wafv2 ip sets.

ISSUE TYPE
  • New Module Pull Request
COMPONENT NAME

wafv2_ip_set
wafv2_ip_set_info

@ansibullbot
Copy link

cc @jillr @s-hertel @tremble @wimnat
click here for bot help

@ansibullbot ansibullbot added community_review integration tests/integration module module needs_triage new_module New module new_plugin New plugin plugins plugin (any type) tests tests labels Mar 1, 2021
@markuman
Copy link
Member Author

markuman commented Mar 1, 2021

aws terminator needs rw access to wafv2

botocore.exceptions.ClientError: An error occurred (AccessDeniedException) when calling the ListIPSets operation: User: arn:aws:sts::966509639900:assumed-role/ansible-core-ci-test-prod/prod=shippable=ansible-collections=community.aws=1701.19 is not authorized to perform: wafv2:ListIPSets on resource: arn:aws:wafv2:us-east-1:966509639900:regional/ipset/*

@ansibullbot ansibullbot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR and removed community_review labels Mar 1, 2021
@markuman markuman mentioned this pull request Mar 1, 2021
Merged
@markuman
Copy link
Member Author

There are failures that are not related with my PR

TASK [lambda : parallel lambda creation 1/4] ***********************************

jillr
jillr reviewed Mar 18, 2021
View reviewed changes
Copy link
Contributor

@jillr jillr left a comment

Choose a reason for hiding this comment

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

This PR will need a changelog.

meta/runtime.yml is causing ci_complete to be run, if you have issues with the tests timing out or exceeding request limits give me a ping on irc and I can rerun them (a few are still failing now, but they're nothing to do with this change)

@markuman markuman requested a review from jillr March 19, 2021 20:10
@jillr
Copy link
Contributor

jillr commented Mar 19, 2021

recheck

jillr
jillr approved these changes Mar 19, 2021
View reviewed changes
Copy link
Contributor

@jillr jillr left a comment

Choose a reason for hiding this comment

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

lgtm when tests are passing

@markuman
Copy link
Member Author

recheck

@markuman
Copy link
Member Author

lgtm when tests are passing

there are still so many failures which are not related to this PR. How can we made them pass?

@markuman
Copy link
Member Author

Required: ansible/ansible#73975 for pass 2.9 integration tests

@s-hertel
Copy link
Collaborator

s-hertel commented Apr 1, 2021

Thanks for putting so much work into these new modules, they look great 😃 I just wanted to mention that once 2.9 becomes security-only, new modules shouldn't be too much of a hassle to test in CI with YAML anchors + module_defaults actions. It should just be an additional two lines for a new module, like

module_defaults:
  group/aws: &aws_connection_info
    ...
  # Use the fqcn here to work regardless of how the plugin is called and ensure future compatibility
  community.aws.new_module:
    <<: *aws_connection_info

(But don't worry; #73975 will be included in the upcoming final non-only-security 2.9 cycle, it just hasn't been added yet because those get merged in batches by the release engineer)

@tremble
Copy link
Contributor

tremble commented Apr 2, 2021

@s-hertel what worries me about using the YAML Anchors is that this will mean that when someone forgets to update the group in runtime.yml the integration tests won't catch it. This is a really poor experience for collection maintainers.

@tremble
Copy link
Contributor

tremble commented Apr 8, 2021

Test failures are due to known flakes. WAF tests are running cleanly and taking around 10-20 seconds.

@tremble tremble merged commit 27a24c1 into ansible-collections:main Apr 8, 2021
@tremble
Copy link
Contributor

tremble commented Apr 8, 2021

Thanks for your work on this @markuman sorry it's taken a while to get everything merged.

alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request Jul 19, 2021
@markuman
add wafv2_ip_set module (ansible-collections#449)
485c819 
* add wafv2 ip set module
* expand meta/runtime with wafv2_ip_set modules
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request Jul 19, 2021
@markuman
add wafv2_ip_set module (ansible-collections#449)
fc8f7bc 
* add wafv2 ip set module
* expand meta/runtime with wafv2_ip_set modules
danielcotton pushed a commit to danielcotton/community.aws that referenced this pull request Nov 23, 2021
@markuman @danielcotton
add wafv2_ip_set module (ansible-collections#449)
3e61d18 
* add wafv2 ip set module
* expand meta/runtime with wafv2_ip_set modules
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request May 25, 2022
@tremble
Add botocore requirements to s3_bucket ownership control management (a…
e1b2d7b 
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jillr jillr jillr approved these changes

Assignees
No one assigned
Labels
integration tests/integration module module needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR new_module New module new_plugin New plugin plugins plugin (any type) tests tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@markuman @ansibullbot @jillr @s-hertel @tremble