wafv2_web_acl - fix return values (#1216)

wafv2_web_acl - fix return values

SUMMARY

split integration tests from full wafv2 tests
relax botocore requirement to bare minimum required
return web acl info on update
consistently return web acl info as described in documentation (create would nest it under "web_acl")
fix "changed" value when description not specified

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
wafv2_web_acl
ADDITIONAL INFORMATION

Reviewed-by: Joseph Torcasso <None>
Reviewed-by: Alina Buzachis <None>
Reviewed-by: Mark Chappell <None>
(cherry picked from commit 01f3274)

changelogs/fragments/1216-wafv2_web_acl-return.yml

@@ -0,0 +1,5 @@
+bugfixes:
+- wafv2_web_acl - consistently return web ACL info as described in module documentation (https://github.com/ansible-collections/community.aws/pull/1216).
+- wafv2_web_acl - fix ``changed`` status when description not specified (https://github.com/ansible-collections/community.aws/pull/1216).
+minor_changes:
+- wafv2_web_acl - relax botocore requirement to bare minimum required (https://github.com/ansible-collections/community.aws/pull/1216).

plugins/modules/wafv2_web_acl.py

@@ -93,7 +93,7 @@
         - A map of custom response keys and content bodies. Define response bodies here and reference them in the rules by providing
         - the key of the body dictionary element.
         - Each element must have a unique dict key and in the dict two keys for I(content_type) and I(content).
-        - Requires botocore >= 1.21.0
+        - Requires botocore >= 1.20.40
       type: dict
       version_added: 3.1.0
     purge_rules:
@@ -341,7 +341,6 @@ def update(self, default_action, description, rules, sampled_requests, cloudwatc
             'Scope': self.scope,
             'Id': self.id,
             'DefaultAction': default_action,
-            'Description': description,
             'Rules': rules,
             'VisibilityConfig': {
                 'SampledRequestsEnabled': sampled_requests,
@@ -351,14 +350,19 @@ def update(self, default_action, description, rules, sampled_requests, cloudwatc
             'LockToken': self.locktoken
         }
 
+        if description:
+            req_obj['Description'] = description
+
         if custom_response_bodies:
             req_obj['CustomResponseBodies'] = custom_response_bodies
 
         try:
             response = self.wafv2.update_web_acl(**req_obj)
         except (BotoCoreError, ClientError) as e:
             self.fail_json_aws(e, msg="Failed to update wafv2 web acl.")
-        return response
+
+        self.existing_acl, self.id, self.locktoken = self.get_web_acl()
+        return self.existing_acl
 
     def remove(self):
         try:
@@ -433,6 +437,18 @@ def create(self, default_action, rules, sampled_requests, cloudwatch_metrics, me
         return self.existing_acl
 
 
+def format_result(result):
+
+    # We were returning details of the Web ACL inside a "web_acl"  parameter on
+    # creation, keep returning it to avoid breaking existing playbooks, but also
+    # return what the docs said we return (and returned when no change happened)
+    retval = dict(result)
+    if "WebACL" in retval:
+        retval.update(retval["WebACL"])
+
+    return camel_dict_to_snake_dict(retval, ignore_list=['tags'])
+
+
 def main():
 
     arg_spec = dict(
@@ -471,7 +487,7 @@ def main():
 
     custom_response_bodies = module.params.get("custom_response_bodies")
     if custom_response_bodies:
-        module.require_botocore_at_least('1.21.0', reason='to set custom response bodies')
+        module.require_botocore_at_least('1.20.40', reason='to set custom response bodies')
         custom_response_bodies = {}
 
         for custom_name, body in module.params.get("custom_response_bodies").items():
@@ -497,8 +513,8 @@ def main():
     if state == 'present':
         if web_acl.get():
             change, rules = compare_priority_rules(web_acl.get().get('WebACL').get('Rules'), rules, purge_rules, state)
-            change = change or web_acl.get().get('WebACL').get('Description') != description
-            change = change or web_acl.get().get('WebACL').get('DefaultAction') != default_action
+            change = change or (description and web_acl.get().get('WebACL').get('Description') != description)
+            change = change or (default_action and web_acl.get().get('WebACL').get('DefaultAction') != default_action)
 
             if change and not check_mode:
                 retval = web_acl.update(
@@ -548,7 +564,7 @@ def main():
                 if not check_mode:
                     retval = web_acl.remove()
 
-    module.exit_json(changed=change, **camel_dict_to_snake_dict(retval))
+    module.exit_json(changed=change, **format_result(retval))
 
 
 if __name__ == '__main__':

tests/integration/targets/wafv2_web_acl/aliases

@@ -0,0 +1,3 @@
+cloud/aws
+
+wafv2_web_acl_info

tests/integration/targets/wafv2_web_acl/defaults/main.yml

@@ -0,0 +1,2 @@
+---
+web_acl_name: '{{ tiny_prefix }}-web-acl'

tests/integration/targets/wafv2_web_acl/meta/main.yml

@@ -0,0 +1,4 @@
+dependencies:
+  - role: setup_botocore_pip
+    vars:
+      botocore_version: "1.20.40"

tests/integration/targets/wafv2_web_acl/tasks/description.yml

@@ -0,0 +1,131 @@
+- name: Tests relating to setting descriptions on wavf2_web_acl
+  vars:
+    description_one: 'a Description - {{ resource_prefix }}'
+    description_two: 'Another_Description - {{ resource_prefix }}'
+  # Mandatory settings
+  module_defaults:
+    community.aws.wafv2_web_acl:
+      name: '{{ web_acl_name }}'
+      state: present
+      scope: REGIONAL
+      purge_rules: no
+      rules: []
+      default_action: Allow
+    community.aws.wafv2_web_acl_info:
+      name: '{{ web_acl_name }}'
+      scope: REGIONAL
+  block:
+
+  - name: test setting description wafv2_web_acl (check mode)
+    wafv2_web_acl:
+      description: '{{ description_one }}'
+    register: update_result
+    check_mode: yes
+  - name: assert that update succeeded
+    assert:
+      that:
+      - update_result is changed
+
+  - name: test setting description wafv2_web_acl
+    wafv2_web_acl:
+      description: '{{ description_one }}'
+    register: update_result
+  - name: assert that update succeeded
+    assert:
+      that:
+      - update_result is changed
+      - update_result.description == description_one
+
+  - name: test setting description wafv2_web_acl - idempotency (check mode)
+    wafv2_web_acl:
+      description: '{{ description_one }}'
+    register: update_result
+    check_mode: yes
+  - name: assert that update succeeded
+    assert:
+      that:
+      - update_result is not changed
+
+  - name: test setting description wafv2_web_acl - idempotency
+    wafv2_web_acl:
+      description: '{{ description_one }}'
+    register: update_result
+  - name: assert that update succeeded
+    assert:
+      that:
+      - update_result is not changed
+      - update_result.description == description_one
+
+  ###
+
+  - name: test updating description on wafv2_web_acl (check mode)
+    wafv2_web_acl:
+      description: '{{ description_two }}'
+    register: update_result
+    check_mode: yes
+  - name: assert that update succeeded
+    assert:
+      that:
+      - update_result is changed
+
+  - name: test updating description on wafv2_web_acl
+    wafv2_web_acl:
+      description: '{{ description_two }}'
+    register: update_result
+  - name: assert that update succeeded
+    assert:
+      that:
+      - update_result is changed
+      - update_result.description == description_two
+
+  - name: test updating description on wafv2_web_acl - idempotency (check mode)
+    wafv2_web_acl:
+      description: '{{ description_two }}'
+    register: update_result
+    check_mode: yes
+  - name: assert that update succeeded
+    assert:
+      that:
+      - update_result is not changed
+
+  - name: test updating description on wafv2_web_acl - idempotency
+    wafv2_web_acl:
+      description: '{{ description_two }}'
+    register: update_result
+  - name: assert that update succeeded
+    assert:
+      that:
+      - update_result is not changed
+      - update_result.description == description_two
+
+  ###
+
+  - name: test that wafv2_web_acl_info returns the description
+    wafv2_web_acl_info:
+    register: tag_info
+  - name: assert description present
+    assert:
+      that:
+      - tag_info.description == description_two
+
+  ###
+
+  - name: test no description param wafv2_web_acl (check mode)
+    wafv2_web_acl: {}
+    register: update_result
+    check_mode: yes
+  - name: assert no change
+    assert:
+      that:
+      - update_result is not changed
+      - update_result.description == description_two
+
+
+  - name: test no description param wafv2_web_acl
+    wafv2_web_acl: {}
+    register: update_result
+  - name: assert no change
+    assert:
+      that:
+      - update_result is not changed
+      - update_result.description == description_two