From b1c4d6310da71a1aa7532c99470b9b41656ee1e9 Mon Sep 17 00:00:00 2001 From: Sloane Hertel Date: Wed, 30 May 2018 13:43:02 -0400 Subject: [PATCH] Fix broken aws_ses_identity test - backport #39560 (#40350) * Fix failing aws_ses_identity integration tests (#39560) * Fix failing aws_ses_identity integration tests Reduce boilerplate with yaml anchor * remove unstable test alias * Update feedback forwarding check to use desired state rather than repeated API calls. (cherry picked from commit 571c183f598845000ae4d8c1d04a53343734e42b) * changelog --- ...eedback_forwarding_without_sns_topics.yaml | 3 + .../modules/cloud/amazon/aws_ses_identity.py | 19 +- .../targets/aws_ses_identity/aliases | 1 - .../targets/aws_ses_identity/tasks/main.yaml | 226 +++++++++--------- 4 files changed, 125 insertions(+), 124 deletions(-) create mode 100644 changelogs/fragments/aws_ses_identity_require_feedback_forwarding_without_sns_topics.yaml diff --git a/changelogs/fragments/aws_ses_identity_require_feedback_forwarding_without_sns_topics.yaml b/changelogs/fragments/aws_ses_identity_require_feedback_forwarding_without_sns_topics.yaml new file mode 100644 index 00000000000..b2579ef66e2 --- /dev/null +++ b/changelogs/fragments/aws_ses_identity_require_feedback_forwarding_without_sns_topics.yaml @@ -0,0 +1,3 @@ +--- +bugfixes: + - To disable feedback forwarding with aws_ses_identity require SNS topics to handle bounces and complaints. diff --git a/lib/ansible/modules/cloud/amazon/aws_ses_identity.py b/lib/ansible/modules/cloud/amazon/aws_ses_identity.py index 9ce1a73309d..5e869a76f95 100644 --- a/lib/ansible/modules/cloud/amazon/aws_ses_identity.py +++ b/lib/ansible/modules/cloud/amazon/aws_ses_identity.py @@ -309,8 +309,15 @@ def get_identity_notifications(connection, module, identity, retries=0, retryDel return notification_attributes[identity] -def update_notification_topic(connection, module, identity, identity_notifications, notification_type): +def desired_topic(module, notification_type): arg_dict = module.params.get(notification_type.lower() + '_notifications') + if arg_dict: + return arg_dict.get('topic', None) + else: + return None + + +def update_notification_topic(connection, module, identity, identity_notifications, notification_type): topic_key = notification_type + 'Topic' if identity_notifications is None: # If there is no configuration for notifications cannot be being sent to topics @@ -325,10 +332,7 @@ def update_notification_topic(connection, module, identity, identity_notificatio # included but best to be defensive current = None - if arg_dict is not None and 'topic' in arg_dict: - required = arg_dict['topic'] - else: - required = None + required = desired_topic(module, notification_type) if current != required: call_and_handle_errors( @@ -375,6 +379,11 @@ def update_notification_topic_headers(connection, module, identity, identity_not def update_feedback_forwarding(connection, module, identity, identity_notifications): + if module.params.get('feedback_forwarding') is False: + if not (desired_topic(module, 'Bounce') and desired_topic(module, 'Complaint')): + module.fail_json(msg="Invalid Parameter Value 'False' for 'feedback_forwarding'. AWS requires " + "feedback forwarding to be enabled unless bounces and complaints are handled by SNS topics") + if identity_notifications is None: # AWS requires feedback forwarding to be enabled unless bounces and complaints # are being handled by SNS topics. So in the absence of identity_notifications diff --git a/test/integration/targets/aws_ses_identity/aliases b/test/integration/targets/aws_ses_identity/aliases index 61d921b0938..d6ae2f116bc 100644 --- a/test/integration/targets/aws_ses_identity/aliases +++ b/test/integration/targets/aws_ses_identity/aliases @@ -1,3 +1,2 @@ cloud/aws posix/ci/cloud/group4/aws -unstable diff --git a/test/integration/targets/aws_ses_identity/tasks/main.yaml b/test/integration/targets/aws_ses_identity/tasks/main.yaml index 16d778e6b21..e860a7babac 100644 --- a/test/integration/targets/aws_ses_identity/tasks/main.yaml +++ b/test/integration/targets/aws_ses_identity/tasks/main.yaml @@ -1,15 +1,21 @@ --- # ============================================================ +- name: set up aws connection info + set_fact: + aws_connection_info: &aws_connection_info + aws_access_key: "{{ aws_access_key }}" + aws_secret_key: "{{ aws_secret_key }}" + security_token: "{{ security_token }}" + region: "{{ aws_region }}" + no_log: yes + - name: test register email identity block: - name: register email identity aws_ses_identity: identity: "{{ email_identity }}" state: present - region: "{{ ec2_region }}" - aws_access_key: "{{ ec2_access_key }}" - aws_secret_key: "{{ ec2_secret_key }}" - security_token: "{{security_token}}" + <<: *aws_connection_info register: result - name: assert changed is True assert: @@ -23,10 +29,7 @@ aws_ses_identity: identity: "{{ email_identity }}" state: absent - region: "{{ ec2_region }}" - aws_access_key: "{{ ec2_access_key }}" - aws_secret_key: "{{ ec2_secret_key }}" - security_token: "{{security_token}}" + <<: *aws_connection_info # ============================================================ - name: test register domain identity block: @@ -34,10 +37,7 @@ aws_ses_identity: identity: "{{ domain_identity }}" state: present - region: "{{ ec2_region }}" - aws_access_key: "{{ ec2_access_key }}" - aws_secret_key: "{{ ec2_secret_key }}" - security_token: "{{security_token}}" + <<: *aws_connection_info register: result - name: assert changed is True assert: @@ -55,10 +55,7 @@ aws_ses_identity: identity: "{{ domain_identity }}" state: absent - region: "{{ ec2_region }}" - aws_access_key: "{{ ec2_access_key }}" - aws_secret_key: "{{ ec2_secret_key }}" - security_token: "{{security_token}}" + <<: *aws_connection_info # ============================================================ - name: test email_identity unchanged when already existing block: @@ -66,18 +63,12 @@ aws_ses_identity: identity: "{{ email_identity }}" state: present - region: "{{ ec2_region }}" - aws_access_key: "{{ ec2_access_key }}" - aws_secret_key: "{{ ec2_secret_key }}" - security_token: "{{security_token}}" + <<: *aws_connection_info - name: duplicate register identity aws_ses_identity: identity: "{{ email_identity }}" state: present - region: "{{ ec2_region }}" - aws_access_key: "{{ ec2_access_key }}" - aws_secret_key: "{{ ec2_secret_key }}" - security_token: "{{security_token}}" + <<: *aws_connection_info register: result - name: assert changed is False assert: @@ -91,10 +82,7 @@ aws_ses_identity: identity: "{{ email_identity }}" state: absent - region: "{{ ec2_region }}" - aws_access_key: "{{ ec2_access_key }}" - aws_secret_key: "{{ ec2_secret_key }}" - security_token: "{{security_token}}" + <<: *aws_connection_info # ============================================================ - name: test domain_identity unchanged when already existing block: @@ -102,18 +90,12 @@ aws_ses_identity: identity: "{{ domain_identity }}" state: present - region: "{{ ec2_region }}" - aws_access_key: "{{ ec2_access_key }}" - aws_secret_key: "{{ ec2_secret_key }}" - security_token: "{{security_token}}" + <<: *aws_connection_info - name: duplicate register identity aws_ses_identity: identity: "{{ domain_identity }}" state: present - region: "{{ ec2_region }}" - aws_access_key: "{{ ec2_access_key }}" - aws_secret_key: "{{ ec2_secret_key }}" - security_token: "{{security_token}}" + <<: *aws_connection_info register: result - name: assert changed is False assert: @@ -127,19 +109,13 @@ aws_ses_identity: identity: "{{ domain_identity }}" state: absent - region: "{{ ec2_region }}" - aws_access_key: "{{ ec2_access_key }}" - aws_secret_key: "{{ ec2_secret_key }}" - security_token: "{{security_token}}" + <<: *aws_connection_info # ============================================================ - name: remove non-existent email identity aws_ses_identity: identity: "{{ email_identity }}" state: absent - region: "{{ ec2_region }}" - aws_access_key: "{{ ec2_access_key }}" - aws_secret_key: "{{ ec2_secret_key }}" - security_token: "{{security_token}}" + <<: *aws_connection_info register: result - name: assert changed is False assert: @@ -150,10 +126,7 @@ aws_ses_identity: identity: "{{ domain_identity }}" state: absent - region: "{{ ec2_region }}" - aws_access_key: "{{ ec2_access_key }}" - aws_secret_key: "{{ ec2_secret_key }}" - security_token: "{{security_token}}" + <<: *aws_connection_info register: result - name: assert changed is False assert: @@ -166,10 +139,7 @@ sns_topic: name: "{{ notification_queue_name }}-{{ item }}" state: present - region: "{{ ec2_region }}" - aws_access_key: "{{ ec2_access_key }}" - aws_secret_key: "{{ ec2_secret_key }}" - security_token: "{{security_token}}" + <<: *aws_connection_info register: topic_info with_items: - bounce @@ -185,10 +155,7 @@ topic: "{{ topic_info.results[1].sns_arn }}" delivery_notifications: topic: "{{ topic_info.results[2].sns_arn }}" - region: "{{ ec2_region }}" - aws_access_key: "{{ ec2_access_key }}" - aws_secret_key: "{{ ec2_secret_key }}" - security_token: "{{security_token}}" + <<: *aws_connection_info register: result - name: assert notification settings assert: @@ -207,10 +174,7 @@ sns_topic: name: "{{ notification_queue_name }}-{{ item }}" state: absent - region: "{{ ec2_region }}" - aws_access_key: "{{ ec2_access_key }}" - aws_secret_key: "{{ ec2_secret_key }}" - security_token: "{{security_token}}" + <<: *aws_connection_info with_items: - bounce - complaint @@ -219,10 +183,7 @@ aws_ses_identity: identity: "{{ email_identity }}" state: absent - region: "{{ ec2_region }}" - aws_access_key: "{{ ec2_access_key }}" - aws_secret_key: "{{ ec2_secret_key }}" - security_token: "{{security_token}}" + <<: *aws_connection_info # ============================================================ - name: test change notification queues after create block: @@ -230,10 +191,7 @@ sns_topic: name: "{{ notification_queue_name }}-{{ item }}" state: present - region: "{{ ec2_region }}" - aws_access_key: "{{ ec2_access_key }}" - aws_secret_key: "{{ ec2_secret_key }}" - security_token: "{{security_token}}" + <<: *aws_connection_info register: topic_info with_items: - bounce @@ -243,10 +201,7 @@ aws_ses_identity: identity: "{{ email_identity }}" state: present - region: "{{ ec2_region }}" - aws_access_key: "{{ ec2_access_key }}" - aws_secret_key: "{{ ec2_secret_key }}" - security_token: "{{security_token}}" + <<: *aws_connection_info - name: set notification topics aws_ses_identity: identity: "{{ email_identity }}" @@ -257,10 +212,7 @@ topic: "{{ topic_info.results[1].sns_arn }}" delivery_notifications: topic: "{{ topic_info.results[2].sns_arn }}" - region: "{{ ec2_region }}" - aws_access_key: "{{ ec2_access_key }}" - aws_secret_key: "{{ ec2_secret_key }}" - security_token: "{{security_token}}" + <<: *aws_connection_info register: result - name: assert changed is True assert: @@ -277,10 +229,7 @@ sns_topic: name: "{{ notification_queue_name }}-{{ item }}" state: absent - region: "{{ ec2_region }}" - aws_access_key: "{{ ec2_access_key }}" - aws_secret_key: "{{ ec2_secret_key }}" - security_token: "{{security_token}}" + <<: *aws_connection_info with_items: - bounce - complaint @@ -289,10 +238,7 @@ aws_ses_identity: identity: "{{ email_identity }}" state: absent - region: "{{ ec2_region }}" - aws_access_key: "{{ ec2_access_key }}" - aws_secret_key: "{{ ec2_secret_key }}" - security_token: "{{security_token}}" + <<: *aws_connection_info # ============================================================ - name: test include headers on notification queues block: @@ -306,10 +252,7 @@ include_headers: Yes delivery_notifications: include_headers: Yes - region: "{{ ec2_region }}" - aws_access_key: "{{ ec2_access_key }}" - aws_secret_key: "{{ ec2_secret_key }}" - security_token: "{{security_token}}" + <<: *aws_connection_info register: result - name: assert notification headers enabled assert: @@ -322,10 +265,7 @@ aws_ses_identity: identity: "{{ email_identity }}" state: absent - region: "{{ ec2_region }}" - aws_access_key: "{{ ec2_access_key }}" - aws_secret_key: "{{ ec2_secret_key }}" - security_token: "{{security_token}}" + <<: *aws_connection_info # ============================================================ - name: test disable feedback forwarding block: @@ -333,10 +273,7 @@ sns_topic: name: "{{ notification_queue_name }}-{{ item }}" state: present - region: "{{ ec2_region }}" - aws_access_key: "{{ ec2_access_key }}" - aws_secret_key: "{{ ec2_secret_key }}" - security_token: "{{security_token}}" + <<: *aws_connection_info register: topic_info with_items: - bounce @@ -350,10 +287,7 @@ complaint_notifications: topic: "{{ topic_info.results[1].sns_arn }}" feedback_forwarding: No - region: "{{ ec2_region }}" - aws_access_key: "{{ ec2_access_key }}" - aws_secret_key: "{{ ec2_secret_key }}" - security_token: "{{security_token}}" + <<: *aws_connection_info register: result - name: assert feedback_forwarding == False assert: @@ -364,10 +298,7 @@ sns_topic: name: "{{ notification_queue_name }}-{{ item }}" state: absent - region: "{{ ec2_region }}" - aws_access_key: "{{ ec2_access_key }}" - aws_secret_key: "{{ ec2_secret_key }}" - security_token: "{{security_token}}" + <<: *aws_connection_info with_items: - bounce - complaint @@ -375,10 +306,7 @@ aws_ses_identity: identity: "{{ email_identity }}" state: absent - region: "{{ ec2_region }}" - aws_access_key: "{{ ec2_access_key }}" - aws_secret_key: "{{ ec2_secret_key }}" - security_token: "{{security_token}}" + <<: *aws_connection_info # ============================================================ - name: test disable feedback forwarding fails if no topics block: @@ -387,22 +315,84 @@ identity: "{{ domain_identity }}" state: present feedback_forwarding: No - region: "{{ ec2_region }}" - aws_access_key: "{{ ec2_access_key }}" - aws_secret_key: "{{ ec2_secret_key }}" - security_token: "{{security_token}}" + <<: *aws_connection_info register: result failed_when: result.failed == False - - name: assert error.code == InvalidParameterValue + - name: assert error message starts with "Invalid Parameter Value" assert: that: - - result.error.code == 'InvalidParameterValue' + - '"Invalid Parameter Value" in result.msg' always: - name: cleanup identity aws_ses_identity: identity: "{{ domain_identity }}" state: absent - region: "{{ ec2_region }}" - aws_access_key: "{{ ec2_access_key }}" - aws_secret_key: "{{ ec2_secret_key }}" - security_token: "{{security_token}}" + <<: *aws_connection_info +# ============================================================ +- name: test disable feedback forwarding fails if no complaint topic + block: + - name: test topic + sns_topic: + name: "{{ notification_queue_name }}-bounce" + state: present + <<: *aws_connection_info + register: topic_info + - name: register email identity + aws_ses_identity: + identity: "{{ email_identity }}" + state: present + bounce_notifications: + topic: "{{ topic_info.sns_arn }}" + feedback_forwarding: No + <<: *aws_connection_info + register: result + failed_when: result.failed == False + - name: assert error message starts with "Invalid Parameter Value" + assert: + that: + - '"Invalid Parameter Value" in result.msg' + always: + - name: cleanup topics + sns_topic: + name: "{{ notification_queue_name }}-bounce" + state: absent + <<: *aws_connection_info + - name: cleanup identity + aws_ses_identity: + identity: "{{ email_identity }}" + state: absent + <<: *aws_connection_info +# ============================================================ +- name: test disable feedback forwarding fails if no bounce topic + block: + - name: test topic + sns_topic: + name: "{{ notification_queue_name }}-complaint" + state: present + <<: *aws_connection_info + register: topic_info + - name: register email identity + aws_ses_identity: + identity: "{{ email_identity }}" + state: present + complaint_notifications: + topic: "{{ topic_info.sns_arn }}" + feedback_forwarding: No + <<: *aws_connection_info + register: result + failed_when: result.failed == False + - name: assert error message starts with "Invalid Parameter Value" + assert: + that: + - '"Invalid Parameter Value" in result.msg' + always: + - name: cleanup topics + sns_topic: + name: "{{ notification_queue_name }}-complaint" + state: absent + <<: *aws_connection_info + - name: cleanup identity + aws_ses_identity: + identity: "{{ email_identity }}" + state: absent + <<: *aws_connection_info