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.

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

wafv2_ip_set - add support for updating tags #1205

Conversation

tremble
Copy link
Contributor

@tremble tremble commented Jun 3, 2022

SUMMARY
  • Added support for purge_tags
  • Added tags to return values
  • Added support for updating tags
  • Moved to common docs_fragment for tagging
ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

plugins/module_utils/wafv2.py
plugins/modules/wafv2_ip_set.py
plugins/modules/wafv2_ip_set_info.py

ADDITIONAL INFORMATION

Depends on : mattclay/aws-terminator#213

@github-actions
Copy link

github-actions bot commented Jun 3, 2022

Docs Build 📝

Thank you for contribution!✨

This PR has been merged and your docs changes will be incorporated when they are next published.

@ansibullbot
Copy link

@ansibullbot ansibullbot added community_review feature This issue/PR relates to a feature request integration tests/integration module module needs_triage plugins plugin (any type) tests tests labels Jun 3, 2022
@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

ansible-galaxy-importer FAILURE in 5m 22s (non-voting)
✔️ build-ansible-collection SUCCESS in 5m 42s
✔️ ansible-test-sanity-docker-devel SUCCESS in 10m 35s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 10m 42s
✔️ ansible-test-sanity-docker-stable-2.9 SUCCESS in 13m 01s
✔️ ansible-test-sanity-docker-stable-2.11 SUCCESS in 10m 06s
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 11m 08s
✔️ ansible-test-units-community-aws-python38 SUCCESS in 6m 34s
✔️ ansible-test-units-community-aws-python39 SUCCESS in 7m 02s
✔️ ansible-test-splitter SUCCESS in 2m 45s
integration-community.aws-1 FAILURE in 5m 27s
⚠️ integration-community.aws-2 SKIPPED
⚠️ integration-community.aws-3 SKIPPED
⚠️ integration-community.aws-4 SKIPPED
⚠️ integration-community.aws-5 SKIPPED
⚠️ integration-community.aws-6 SKIPPED
⚠️ integration-community.aws-7 SKIPPED
⚠️ integration-community.aws-8 SKIPPED
⚠️ integration-community.aws-9 SKIPPED
⚠️ integration-community.aws-10 SKIPPED
⚠️ integration-community.aws-11 SKIPPED
⚠️ integration-community.aws-12 SKIPPED
⚠️ integration-community.aws-13 SKIPPED

@gravesm
Copy link
Member

gravesm commented Jun 3, 2022

recheck

@tremble tremble force-pushed the tagging/purge/wafv2_ip_set branch from bd3fc52 to d3e0d15 Compare June 3, 2022 12:38
@markuman
Copy link
Member

markuman commented Jun 3, 2022

there should be a note: in the docs section, that purge_tags is supported since 4.0.0.
And it must be set to defaults(no) in the first release

tags=dict(type='dict'),
purge_addresses=dict(type='bool', default=True)
tags=dict(type='dict', aliases=['resource_tags']),
purge_tags=dict(type='bool', default=True),
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
purge_tags=dict(type='bool', default=True),
purge_tags=dict(type='bool', default=False),

in the first release

@tremble
Copy link
Contributor Author

tremble commented Jun 3, 2022

there should be a note: in the docs section, that purge_tags is supported since 4.0.0. And it must be set to defaults(no) in the first release

I was in two minds over the default value. Since it didn't seem to set tags at all on update previously, I went with defaulting to True...

@markuman
Copy link
Member

markuman commented Jun 3, 2022

there should be a note: in the docs section, that purge_tags is supported since 4.0.0. And it must be set to defaults(no) in the first release

I was in two minds over the default value. Since it didn't seem to set tags at all on update previously, I went with defaulting to True...

Yes good question. This will hit only users that are dealing with boto3 and maybe other orchestration tools like cloudformation....but dunno. better safe than sorry?

@tremble
Copy link
Contributor Author

tremble commented Jun 3, 2022

Yes good question. This will hit only users that are dealing with boto3 and maybe other orchestration tools like cloudformation....but dunno. better safe than sorry?

unless tags is explicitly set in the task ensure_wafv2_tags will do nothing, so we should be good...

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

ansible-galaxy-importer FAILURE in 4m 44s (non-voting)
✔️ build-ansible-collection SUCCESS in 4m 48s
✔️ ansible-test-sanity-docker-devel SUCCESS in 9m 26s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 9m 21s
✔️ ansible-test-sanity-docker-stable-2.9 SUCCESS in 11m 29s
✔️ ansible-test-sanity-docker-stable-2.11 SUCCESS in 10m 08s
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 12m 10s
✔️ ansible-test-units-community-aws-python38 SUCCESS in 6m 05s
✔️ ansible-test-units-community-aws-python39 SUCCESS in 5m 46s
✔️ ansible-test-splitter SUCCESS in 2m 26s
✔️ integration-community.aws-1 SUCCESS in 6m 41s
⚠️ integration-community.aws-2 SKIPPED
⚠️ integration-community.aws-3 SKIPPED
⚠️ integration-community.aws-4 SKIPPED
⚠️ integration-community.aws-5 SKIPPED
⚠️ integration-community.aws-6 SKIPPED
⚠️ integration-community.aws-7 SKIPPED
⚠️ integration-community.aws-8 SKIPPED
⚠️ integration-community.aws-9 SKIPPED
⚠️ integration-community.aws-10 SKIPPED
⚠️ integration-community.aws-11 SKIPPED
⚠️ integration-community.aws-12 SKIPPED
⚠️ integration-community.aws-13 SKIPPED

@tremble tremble added the mergeit Merge the PR (SoftwareFactory) label Jun 3, 2022
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).

ansible-galaxy-importer FAILURE in 4m 18s (non-voting)
✔️ build-ansible-collection SUCCESS in 4m 58s
✔️ ansible-test-sanity-docker-devel SUCCESS in 11m 05s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 10m 09s
✔️ ansible-test-sanity-docker-stable-2.9 SUCCESS in 12m 30s
✔️ ansible-test-sanity-docker-stable-2.11 SUCCESS in 10m 39s
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 11m 23s
✔️ ansible-test-units-community-aws-python38 SUCCESS in 5m 36s
✔️ ansible-test-units-community-aws-python39 SUCCESS in 5m 51s
✔️ ansible-test-splitter SUCCESS in 2m 35s
✔️ integration-community.aws-1 SUCCESS in 8m 02s
⚠️ integration-community.aws-2 SKIPPED
⚠️ integration-community.aws-3 SKIPPED
⚠️ integration-community.aws-4 SKIPPED
⚠️ integration-community.aws-5 SKIPPED
⚠️ integration-community.aws-6 SKIPPED
⚠️ integration-community.aws-7 SKIPPED
⚠️ integration-community.aws-8 SKIPPED
⚠️ integration-community.aws-9 SKIPPED
⚠️ integration-community.aws-10 SKIPPED
⚠️ integration-community.aws-11 SKIPPED
⚠️ integration-community.aws-12 SKIPPED
⚠️ integration-community.aws-13 SKIPPED

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit 36d8398 into ansible-collections:main Jun 3, 2022
@tremble tremble deleted the tagging/purge/wafv2_ip_set branch July 7, 2022 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community_review feature This issue/PR relates to a feature request integration tests/integration mergeit Merge the PR (SoftwareFactory) module module plugins plugin (any type) tests tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants