From 1e3fa7011b5587b83ff830cc32c278b4baff8601 Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Tue, 7 Jun 2022 15:31:37 +0200 Subject: [PATCH] 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 Reviewed-by: Alina Buzachis Reviewed-by: Mark Chappell (cherry picked from commit 01f3274762d475922db6b6c8ccca468cbae92905) --- .../fragments/1216-wafv2_web_acl-return.yml | 5 + plugins/modules/wafv2_web_acl.py | 30 +- .../integration/targets/wafv2_web_acl/aliases | 3 + .../targets/wafv2_web_acl/defaults/main.yml | 2 + .../targets/wafv2_web_acl/meta/main.yml | 4 + .../wafv2_web_acl/tasks/description.yml | 131 ++++ .../targets/wafv2_web_acl/tasks/main.yml | 567 ++++++++++++++++++ 7 files changed, 735 insertions(+), 7 deletions(-) create mode 100644 changelogs/fragments/1216-wafv2_web_acl-return.yml create mode 100644 tests/integration/targets/wafv2_web_acl/aliases create mode 100644 tests/integration/targets/wafv2_web_acl/defaults/main.yml create mode 100644 tests/integration/targets/wafv2_web_acl/meta/main.yml create mode 100644 tests/integration/targets/wafv2_web_acl/tasks/description.yml create mode 100644 tests/integration/targets/wafv2_web_acl/tasks/main.yml diff --git a/changelogs/fragments/1216-wafv2_web_acl-return.yml b/changelogs/fragments/1216-wafv2_web_acl-return.yml new file mode 100644 index 00000000000..d5988d4fe02 --- /dev/null +++ b/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). diff --git a/plugins/modules/wafv2_web_acl.py b/plugins/modules/wafv2_web_acl.py index b11b0872b0e..d225a0ae890 100644 --- a/plugins/modules/wafv2_web_acl.py +++ b/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,6 +350,9 @@ 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 @@ -358,7 +360,9 @@ def update(self, default_action, description, rules, sampled_requests, cloudwatc 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__': diff --git a/tests/integration/targets/wafv2_web_acl/aliases b/tests/integration/targets/wafv2_web_acl/aliases new file mode 100644 index 00000000000..8a25f857b99 --- /dev/null +++ b/tests/integration/targets/wafv2_web_acl/aliases @@ -0,0 +1,3 @@ +cloud/aws + +wafv2_web_acl_info diff --git a/tests/integration/targets/wafv2_web_acl/defaults/main.yml b/tests/integration/targets/wafv2_web_acl/defaults/main.yml new file mode 100644 index 00000000000..804bf557324 --- /dev/null +++ b/tests/integration/targets/wafv2_web_acl/defaults/main.yml @@ -0,0 +1,2 @@ +--- +web_acl_name: '{{ tiny_prefix }}-web-acl' diff --git a/tests/integration/targets/wafv2_web_acl/meta/main.yml b/tests/integration/targets/wafv2_web_acl/meta/main.yml new file mode 100644 index 00000000000..07d8bd6d634 --- /dev/null +++ b/tests/integration/targets/wafv2_web_acl/meta/main.yml @@ -0,0 +1,4 @@ +dependencies: + - role: setup_botocore_pip + vars: + botocore_version: "1.20.40" diff --git a/tests/integration/targets/wafv2_web_acl/tasks/description.yml b/tests/integration/targets/wafv2_web_acl/tasks/description.yml new file mode 100644 index 00000000000..4650d75e4f6 --- /dev/null +++ b/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 diff --git a/tests/integration/targets/wafv2_web_acl/tasks/main.yml b/tests/integration/targets/wafv2_web_acl/tasks/main.yml new file mode 100644 index 00000000000..9e5ed8971d6 --- /dev/null +++ b/tests/integration/targets/wafv2_web_acl/tasks/main.yml @@ -0,0 +1,567 @@ +--- +- module_defaults: + group/aws: + aws_access_key: "{{ aws_access_key }}" + aws_secret_key: "{{ aws_secret_key }}" + security_token: "{{ security_token | default(omit) }}" + region: "{{ aws_region }}" + vars: + ansible_python_interpreter: "{{ botocore_virtualenv_interpreter }}" + block: + + ####################### + ## Create web acl + ####################### + + - name: check_mode create web acl + wafv2_web_acl: + name: "{{ web_acl_name }}" + state: present + description: hallo eins + scope: REGIONAL + default_action: Allow + sampled_requests: no + cloudwatch_metrics: yes + metric_name: blub + rules: + - name: zwei + priority: 2 + action: + block: {} + visibility_config: + sampled_requests_enabled: yes + cloud_watch_metrics_enabled: yes + metric_name: ddos + statement: + xss_match_statement: + field_to_match: + body: {} + text_transformations: + - type: NONE + priority: 0 + - name: admin_protect + priority: 1 + override_action: + none: {} + visibility_config: + sampled_requests_enabled: yes + cloud_watch_metrics_enabled: yes + metric_name: fsd + statement: + managed_rule_group_statement: + vendor_name: AWS + name: AWSManagedRulesAdminProtectionRuleSet + custom_response_bodies: + too_many_requests: + content_type: APPLICATION_JSON + content: '{ message: "Your request has been blocked due to too many HTTP requests coming from your IP" }' + tags: + A: B + C: D + register: out + check_mode: yes + + - name: check_mode verify create + assert: + that: + - out is changed + + - name: Create web acl with custom response bodies + wafv2_web_acl: + name: "{{ resource_prefix }}-acl-with-response-body" + state: present + description: foo + scope: REGIONAL + default_action: Allow + sampled_requests: no + cloudwatch_metrics: no + rules: + - name: rate-limit-per-IP + priority: 1 + action: + block: + custom_response: + response_code: 429 + custom_response_body_key: too_many_requests + statement: + rate_based_statement: + limit: 1000 + aggregate_key_type: IP + visibility_config: + sampled_requests_enabled: yes + cloud_watch_metrics_enabled: no + metric_name: unused + custom_response_bodies: + too_many_requests: + content_type: APPLICATION_JSON + content: '{ message: "Your request has been blocked due to too many HTTP requests coming from your IP" }' + register: acl_with_response_body + + - name: Web acl with custom response bodies verify create + assert: + that: + - acl_with_response_body is changed + - acl_with_response_body.web_acl.rules | count == 1 + - acl_with_response_body.web_acl.custom_response_bodies.too_many_requests is defined + + - name: Update web acl with custom response bodies to remove custom response + wafv2_web_acl: + name: "{{ resource_prefix }}-acl-with-response-body" + state: present + scope: REGIONAL + description: foo + default_action: Allow + sampled_requests: no + cloudwatch_metrics: no + rules: + - name: rate-limit-per-IP + priority: 1 + action: + block: {} + statement: + rate_based_statement: + limit: 1000 + aggregate_key_type: IP + visibility_config: + sampled_requests_enabled: yes + cloud_watch_metrics_enabled: no + metric_name: unused + custom_response_bodies: {} + + # unfortunately the wafv2_web_acl does not return the ACL structure after an update + # hence we have to do another task here using the info module to retrieve the latest state + # of the ACL and then to check it + - name: check if custom response body was really removed + wafv2_web_acl_info: + name: "{{ resource_prefix }}-acl-with-response-body" + scope: REGIONAL + register: acl_without_response_bodies + + - name: Web acl with custom response bodies verify removal of custom response + assert: + that: + - acl_without_response_bodies.custom_response_bodies is undefined + + - name: create web acl + wafv2_web_acl: + name: "{{ web_acl_name }}" + state: present + description: hallo eins + scope: REGIONAL + default_action: Allow + sampled_requests: no + cloudwatch_metrics: yes + metric_name: blub + rules: + - name: zwei + priority: 2 + action: + block: {} + visibility_config: + sampled_requests_enabled: yes + cloud_watch_metrics_enabled: yes + metric_name: ddos + statement: + xss_match_statement: + field_to_match: + body: {} + text_transformations: + - type: NONE + priority: 0 + - name: admin_protect + priority: 1 + override_action: + none: {} + visibility_config: + sampled_requests_enabled: yes + cloud_watch_metrics_enabled: yes + metric_name: fsd + statement: + managed_rule_group_statement: + vendor_name: AWS + name: AWSManagedRulesAdminProtectionRuleSet + - name: rate-limit-per-IP + priority: 3 + action: + block: + custom_response: + response_code: 429 + custom_response_body_key: too_many_requests + statement: + rate_based_statement: + limit: 5000 + aggregate_key_type: IP + visibility_config: + sampled_requests_enabled: yes + cloud_watch_metrics_enabled: yes + metric_name: waf-acl-rule-rate-limit-per-IP + custom_response_bodies: + too_many_requests: + content_type: APPLICATION_JSON + content: '{ message: "Your request has been blocked due to too many HTTP requests coming from your IP" }' + tags: + A: B + C: D + register: ACL + + - name: verify create + assert: + that: + - ACL is changed + - ACL.web_acl.name == web_acl_name + - not ACL.web_acl.visibility_config.sampled_requests_enabled + - ACL.web_acl.rules | count == 3 + - ACL.web_acl.description == 'hallo eins' + + - name: immutable create web acl + wafv2_web_acl: + name: "{{ web_acl_name }}" + state: present + description: hallo eins + scope: REGIONAL + default_action: Allow + sampled_requests: no + cloudwatch_metrics: yes + metric_name: blub + rules: + - name: zwei + priority: 2 + action: + block: {} + visibility_config: + sampled_requests_enabled: yes + cloud_watch_metrics_enabled: yes + metric_name: ddos + statement: + xss_match_statement: + field_to_match: + body: {} + text_transformations: + - type: NONE + priority: 0 + - name: admin_protect + priority: 1 + override_action: + none: {} + visibility_config: + sampled_requests_enabled: yes + cloud_watch_metrics_enabled: yes + metric_name: fsd + statement: + managed_rule_group_statement: + vendor_name: AWS + name: AWSManagedRulesAdminProtectionRuleSet + - name: rate-limit-per-IP + priority: 3 + action: + block: + custom_response: + response_code: 429 + custom_response_body_key: too_many_requests + statement: + rate_based_statement: + limit: 5000 + aggregate_key_type: IP + visibility_config: + sampled_requests_enabled: yes + cloud_watch_metrics_enabled: yes + metric_name: waf-acl-rule-rate-limit-per-IP + custom_response_bodies: + too_many_requests: + content_type: APPLICATION_JSON + content: '{ message: "Your request has been blocked due to too many HTTP requests coming from your IP" }' + tags: + A: B + C: D + register: out + + - name: verify create + assert: + that: + - out is not changed + + ############################### + # test web acl + ############################### + - name: get web acl + wafv2_web_acl_info: + name: "{{ web_acl_name }}" + scope: REGIONAL + register: out + + - name: verify rules + assert: + that: + - out.rules | count == 3 + + - name: change web acl description + wafv2_web_acl: + name: "{{ web_acl_name }}" + state: present + description: hallo eins drei + scope: REGIONAL + default_action: Allow + sampled_requests: no + cloudwatch_metrics: yes + metric_name: blub + rules: + - name: zwei + priority: 2 + action: + block: {} + visibility_config: + sampled_requests_enabled: yes + cloud_watch_metrics_enabled: yes + metric_name: ddos + statement: + xss_match_statement: + field_to_match: + body: {} + text_transformations: + - type: NONE + priority: 0 + - name: admin_protect + priority: 1 + override_action: + none: {} + visibility_config: + sampled_requests_enabled: yes + cloud_watch_metrics_enabled: yes + metric_name: fsd + statement: + managed_rule_group_statement: + vendor_name: AWS + name: AWSManagedRulesAdminProtectionRuleSet + tags: + A: B + C: D + register: out + + - name: verify change + assert: + that: + - out is changed + + + - name: add 1 rules + wafv2_web_acl: + name: "{{ web_acl_name }}" + state: present + description: hallo eins drei + scope: REGIONAL + default_action: Allow + sampled_requests: no + cloudwatch_metrics: yes + metric_name: blub + purge_rules: no + rules: + - name: bla + priority: 8 + override_action: + none: {} + visibility_config: + sampled_requests_enabled: yes + cloud_watch_metrics_enabled: yes + metric_name: fsd + statement: + managed_rule_group_statement: + vendor_name: AWS + name: AWSManagedRulesAdminProtectionRuleSet + tags: + A: B + C: D + register: out + + - name: verify change + assert: + that: + - out is changed + + - name: get web acl + wafv2_web_acl_info: + name: "{{ web_acl_name }}" + scope: REGIONAL + register: out + + - name: verify rules + assert: + that: + - out.rules | count == 3 + + - name: reduce rules to 1 + wafv2_web_acl: + name: "{{ web_acl_name }}" + state: present + description: hallo eins drei + scope: REGIONAL + default_action: Allow + sampled_requests: no + cloudwatch_metrics: yes + metric_name: blub + purge_rules: yes + rules: + - name: admin_protect + priority: 1 + override_action: + none: {} + visibility_config: + sampled_requests_enabled: yes + cloud_watch_metrics_enabled: yes + metric_name: admin_protect + statement: + managed_rule_group_statement: + vendor_name: AWS + name: AWSManagedRulesAdminProtectionRuleSet + tags: + A: B + C: D + register: out + + - name: verify change + assert: + that: + - out is changed + + - name: get web acl + wafv2_web_acl_info: + name: "{{ web_acl_name }}" + scope: REGIONAL + register: out + + - name: verify rules + assert: + that: + - out.rules | count == 1 + + - name: immutable change web acl + wafv2_web_acl: + name: "{{ web_acl_name }}" + state: present + description: hallo eins drei + scope: REGIONAL + default_action: Allow + sampled_requests: no + cloudwatch_metrics: yes + metric_name: blub + rules: + - name: admin_protect + priority: 1 + override_action: + none: {} + visibility_config: + sampled_requests_enabled: yes + cloud_watch_metrics_enabled: yes + metric_name: admin_protect + statement: + managed_rule_group_statement: + vendor_name: AWS + name: AWSManagedRulesAdminProtectionRuleSet + tags: + A: B + C: D + register: out + + - name: verify no change + assert: + that: + - out is not changed + + - name: test geo match statement + wafv2_web_acl: + name: "{{ web_acl_name }}" + state: present + description: hallo eins drei + scope: REGIONAL + default_action: Allow + sampled_requests: no + cloudwatch_metrics: yes + metric_name: blub + purge_rules: yes + rules: + - name: block-germany + priority: 1 + action: + block: {} + visibility_config: + sampled_requests_enabled: yes + cloud_watch_metrics_enabled: yes + metric_name: block-germany + statement: + geo_match_statement: + country_codes: + - DE + tags: + A: B + C: D + register: out + + - name: verify change + assert: + that: + - out is changed + + - name: re-read webacl + wafv2_web_acl_info: + name: "{{ web_acl_name }}" + scope: REGIONAL + register: out + + - name: verify geo match statement + assert: + that: + - out.rules[0].statement.geo_match_statement.country_codes[0] == 'DE' + + - include_tasks: 'description.yml' + + - name: re-read webacl + wafv2_web_acl_info: + name: "{{ web_acl_name }}" + scope: REGIONAL + register: out + + - name: verify geo match statement + assert: + that: + - out.rules[0].statement.geo_match_statement.country_codes[0] == 'DE' + + - name: delete web acl + wafv2_web_acl: + name: "{{ web_acl_name }}" + state: absent + scope: REGIONAL + register: out + + - name: verify change + assert: + that: + - out is changed + + - name: immutable delete web acl + wafv2_web_acl: + name: "{{ web_acl_name }}" + state: absent + scope: REGIONAL + register: out + + - name: verify not change + assert: + that: + - out is not changed + + always: + ################################### + # always delete wafv2 components + ################################### + - name: always delete web acl + wafv2_web_acl: + name: "{{ web_acl_name }}" + state: absent + scope: REGIONAL + ignore_errors: true + + - name: Ensure ACL with response body is removed + wafv2_web_acl: + name: "{{ resource_prefix }}-acl-with-response-body" + state: absent + scope: REGIONAL + ignore_errors: true