Support check mode in aws_ses_identity module (#38422)

* Port aws_ses_identity module to use AnsibleAWSModule

* Support Check Mode in aws_ses_identity

* Add tests for check mode

* Move feedback forwarding parameter check to before any changes are made.

changelogs/fragments/aws_ses_identity_check_mode.yaml

@@ -0,0 +1,2 @@
+minor_changes:
+  - The aws_ses_identity module supports check mode

lib/ansible/modules/cloud/amazon/aws_ses_identity.py

@@ -210,52 +210,19 @@ notification_attributes:
         headers_in_delivery_notifications_enabled:
             description: Whether or not headers are included in messages delivered to the delivery topic.
             type: bool
-error:
-    description: The details of the error response from AWS.
-    returned: on client error from AWS
-    type: complex
-    sample: {
-        "code": "InvalidParameterValue",
-        "message": "Feedback notification topic is not set.",
-        "type": "Sender"
-    }
-    contains:
-        code:
-            description: The AWS error code.
-            type: string
-        message:
-            description: The AWS error message.
-            type: string
-        type:
-            description: The AWS error type.
-            type: string
 '''
 
-from ansible.module_utils.basic import AnsibleModule
+from ansible.module_utils.aws.core import AnsibleAWSModule
-from ansible.module_utils.ec2 import camel_dict_to_snake_dict, ec2_argument_spec, get_aws_connection_info, boto3_conn
+from ansible.module_utils.ec2 import camel_dict_to_snake_dict, AWSRetry
-from ansible.module_utils.ec2 import HAS_BOTO3
 
 import time
 
-import traceback
-
 try:
     from botocore.exceptions import BotoCoreError, ClientError
-    from botocore.config import Config
 except ImportError:
     pass  # caught by imported HAS_BOTO3
 
 
-def call_and_handle_errors(module, method, **kwargs):
-    try:
-        return method(**kwargs)
-    except ClientError as e:
-        module.fail_json(msg=str(e), exception=traceback.format_exc(),
-                         **camel_dict_to_snake_dict(e.response))
-    except BotoCoreError as e:
-        module.fail_json(msg=str(e), exception=traceback.format_exc())
-
-
 def get_verification_attributes(connection, module, identity, retries=0, retryDelay=10):
     # Unpredictably get_identity_verification_attributes doesn't include the identity even when we've
     # just registered it. Suspect this is an eventual consistency issue on AWS side.
@@ -263,7 +230,10 @@ def get_verification_attributes(connection, module, identity, retries=0, retryDe
     # a consistent return from the module.
     # To avoid this we have an internal retry that we use only after registering the identity.
     for attempt in range(0, retries + 1):
-        response = call_and_handle_errors(module, connection.get_identity_verification_attributes, Identities=[identity])
+        try:
+            response = connection.get_identity_verification_attributes(Identities=[identity], aws_retry=True)
+        except (BotoCoreError, ClientError) as e:
+            module.fail_json_aws(e, msg='Failed to retrieve identity verification attributes for {identity}'.format(identity=identity))
         identity_verification = response['VerificationAttributes']
         if identity in identity_verification:
             break
@@ -281,7 +251,10 @@ def get_identity_notifications(connection, module, identity, retries=0, retryDel
     # To avoid this we have an internal retry that we use only when getting the current notification
     # status for return.
     for attempt in range(0, retries + 1):
-        response = call_and_handle_errors(module, connection.get_identity_notification_attributes, Identities=[identity])
+        try:
+            response = connection.get_identity_notification_attributes(Identities=[identity], aws_retry=True)
+        except (BotoCoreError, ClientError) as e:
+            module.fail_json_aws(e, msg='Failed to retrieve identity notification attributes for {identity}'.format(identity=identity))
         notification_attributes = response['NotificationAttributes']
 
         # No clear AWS docs on when this happens, but it appears sometimes identities are not included in
@@ -335,13 +308,14 @@ def update_notification_topic(connection, module, identity, identity_notificatio
     required = desired_topic(module, notification_type)
 
     if current != required:
-        call_and_handle_errors(
+        try:
-            module,
+            if not module.check_mode:
-            connection.set_identity_notification_topic,
+                connection.set_identity_notification_topic(Identity=identity, NotificationType=notification_type, SnsTopic=required, aws_retry=True)
-            Identity=identity,
+        except (BotoCoreError, ClientError) as e:
-            NotificationType=notification_type,
+            module.fail_json_aws(e, msg='Failed to set identity notification topic for {identity} {notification_type}'.format(
-            SnsTopic=required,
+                identity=identity,
-        )
+                notification_type=notification_type,
+            ))
         return True
     return False
 
@@ -367,23 +341,20 @@ def update_notification_topic_headers(connection, module, identity, identity_not
         required = False
 
     if current != required:
-        call_and_handle_errors(
+        try:
-            module,
+            if not module.check_mode:
-            connection.set_identity_headers_in_notifications_enabled,
+                connection.set_identity_headers_in_notifications_enabled(Identity=identity, NotificationType=notification_type, Enabled=required,
-            Identity=identity,
+                                                                         aws_retry=True)
-            NotificationType=notification_type,
+        except (BotoCoreError, ClientError) as e:
-            Enabled=required,
+            module.fail_json_aws(e, msg='Failed to set identity headers in notification for {identity} {notification_type}'.format(
-        )
+                identity=identity,
+                notification_type=notification_type,
+            ))
         return True
     return False
 
 
 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
@@ -400,16 +371,32 @@ def update_feedback_forwarding(connection, module, identity, identity_notificati
     required = module.params.get('feedback_forwarding')
 
     if current != required:
-        call_and_handle_errors(
+        try:
-            module,
+            if not module.check_mode:
-            connection.set_identity_feedback_forwarding_enabled,
+                connection.set_identity_feedback_forwarding_enabled(Identity=identity, ForwardingEnabled=required, aws_retry=True)
-            Identity=identity,
+        except (BotoCoreError, ClientError) as e:
-            ForwardingEnabled=required,
+            module.fail_json_aws(e, msg='Failed to set identity feedback forwarding for {identity}'.format(identity=identity))
-        )
         return True
     return False
 
 
+def create_mock_notifications_response(module):
+    resp = {
+        "ForwardingEnabled": module.params.get('feedback_forwarding'),
+    }
+    for notification_type in ('Bounce', 'Complaint', 'Delivery'):
+        arg_dict = module.params.get(notification_type.lower() + '_notifications')
+        if arg_dict is not None and 'topic' in arg_dict:
+            resp[notification_type + 'Topic'] = arg_dict['topic']
+
+        header_key = 'HeadersIn' + notification_type + 'NotificationsEnabled'
+        if arg_dict is not None and 'include_headers' in arg_dict:
+            resp[header_key] = arg_dict['include_headers']
+        else:
+            resp[header_key] = False
+    return resp
+
+
 def update_identity_notifications(connection, module):
     identity = module.params.get('identity')
     changed = False
@@ -422,20 +409,39 @@ def update_identity_notifications(connection, module):
     changed |= update_feedback_forwarding(connection, module, identity, identity_notifications)
 
     if changed or identity_notifications is None:
-        identity_notifications = get_identity_notifications(connection, module, identity, retries=4)
+        if module.check_mode:
+            identity_notifications = create_mock_notifications_response(module)
+        else:
+            identity_notifications = get_identity_notifications(connection, module, identity, retries=4)
     return changed, identity_notifications
 
 
+def validate_params_for_identity_present(module):
+    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")
+
+
 def create_or_update_identity(connection, module, region, account_id):
     identity = module.params.get('identity')
     changed = False
     verification_attributes = get_verification_attributes(connection, module, identity)
     if verification_attributes is None:
-        if '@' in identity:
+        try:
-            call_and_handle_errors(module, connection.verify_email_identity, EmailAddress=identity)
+            if not module.check_mode:
+                if '@' in identity:
+                    connection.verify_email_identity(EmailAddress=identity, aws_retry=True)
+                else:
+                    connection.verify_domain_identity(Domain=identity, aws_retry=True)
+        except (BotoCoreError, ClientError) as e:
+            module.fail_json_aws(e, msg='Failed to verify identity {identity}'.format(identity=identity))
+        if module.check_mode:
+            verification_attributes = {
+                "VerificationStatus": "Pending",
+            }
         else:
-            call_and_handle_errors(module, connection.verify_domain_identity, Domain=identity)
+            verification_attributes = get_verification_attributes(connection, module, identity, retries=4)
-        verification_attributes = get_verification_attributes(connection, module, identity, retries=4)
         changed = True
     elif verification_attributes['VerificationStatus'] not in ('Pending', 'Success'):
         module.fail_json(msg="Identity " + identity + " in bad status " + verification_attributes['VerificationStatus'],
@@ -466,7 +472,11 @@ def destroy_identity(connection, module):
     changed = False
     verification_attributes = get_verification_attributes(connection, module, identity)
     if verification_attributes is not None:
-        call_and_handle_errors(module, connection.delete_identity, Identity=identity)
+        try:
+            if not module.check_mode:
+                connection.delete_identity(Identity=identity, aws_retry=True)
+        except (BotoCoreError, ClientError) as e:
+            module.fail_json_aws(e, msg='Failed to delete identity {identity}'.format(identity=identity))
         changed = True
 
     module.exit_json(
@@ -475,32 +485,28 @@ def destroy_identity(connection, module):
     )
 
 
-def get_account_id(sts):
+def get_account_id(module):
-    caller_identity = sts.get_caller_identity()
+    sts = module.client('sts')
+    try:
+        caller_identity = sts.get_caller_identity()
+    except (BotoCoreError, ClientError) as e:
+        module.fail_json_aws(e, msg='Failed to retrieve caller identity')
     return caller_identity['Account']
 
 
 def main():
-    argument_spec = ec2_argument_spec()
+    module = AnsibleAWSModule(
-
+        argument_spec={
-    argument_spec.update(
+            "identity": dict(required=True, type='str'),
-        dict(
+            "state": dict(default='present', choices=['present', 'absent']),
-            identity=dict(required=True, type='str'),
+            "bounce_notifications": dict(type='dict'),
-            state=dict(default='present', choices=['present', 'absent']),
+            "complaint_notifications": dict(type='dict'),
-            bounce_notifications=dict(type='dict'),
+            "delivery_notifications": dict(type='dict'),
-            complaint_notifications=dict(type='dict'),
+            "feedback_forwarding": dict(default=True, type='bool'),
-            delivery_notifications=dict(type='dict'),
+        },
-            feedback_forwarding=dict(default=True, type='bool'),
+        supports_check_mode=True,
-        )
     )
 
-    module = AnsibleModule(
-        argument_spec=argument_spec,
-    )
-
-    if not HAS_BOTO3:
-        module.fail_json(msg='boto3 required for this module')
-
     for notification_type in ('bounce', 'complaint', 'delivery'):
         param_name = notification_type + '_notifications'
         arg_dict = module.params.get(param_name)
@@ -509,23 +515,18 @@ def main():
             if extra_keys:
                 module.fail_json(msg='Unexpected keys ' + str(extra_keys) + ' in ' + param_name + ' valid keys are topic or include_headers')
 
-    region, ec2_url, aws_connect_params = get_aws_connection_info(module, boto3=True)
-
-    # Allow up to 10 attempts to call the SES APIs before giving up (9 retries).
     # SES APIs seem to have a much lower throttling threshold than most of the rest of the AWS APIs.
     # Docs say 1 call per second. This shouldn't actually be a big problem for normal usage, but
-    # the ansible build runs multiple instances of the test in parallel.
+    # the ansible build runs multiple instances of the test in parallel that's caused throttling
-    # As a result there are build failures due to throttling that exceeds boto's default retries.
+    # failures so apply a jittered backoff to call SES calls.
-    # The back-off is exponential, so upping the retry attempts allows multiple parallel runs
+    connection = module.client('ses', retry_decorator=AWSRetry.jittered_backoff())
-    # to succeed.
-    boto_core_config = Config(retries={'max_attempts': 9})
-    connection = boto3_conn(module, conn_type='client', resource='ses', region=region, endpoint=ec2_url, config=boto_core_config, **aws_connect_params)
 
     state = module.params.get("state")
 
     if state == 'present':
-        sts = boto3_conn(module, conn_type='client', resource='sts', region=region, endpoint=ec2_url, **aws_connect_params)
+        region = module.params.get('region')
-        account_id = get_account_id(sts)
+        account_id = get_account_id(module)
+        validate_params_for_identity_present(module)
         create_or_update_identity(connection, module, region, account_id)
     else:
         destroy_identity(connection, module)

test/integration/targets/aws_ses_identity/tasks/main.yaml

@@ -111,6 +111,70 @@
         state: absent
         <<: *aws_connection_info
 # ============================================================
+- name: test register email identity check mode
+  block:
+    - name: register email identity check mode
+      aws_ses_identity:
+        identity: "{{ email_identity }}"
+        state: present
+        <<: *aws_connection_info
+      register: result
+      check_mode: True
+
+    - name: assert changed is True
+      assert:
+        that:
+          - result.changed == True
+
+    - import_tasks: assert_defaults.yaml
+      vars:
+        identity: "{{ email_identity }}"
+
+  always:
+    - name: cleanup email identity
+      aws_ses_identity:
+        identity: "{{ email_identity }}"
+        state: absent
+        <<: *aws_connection_info
+      register: result
+
+    - name: assert nothing to clean up since check mode
+      assert:
+        that:
+          - result.changed == False
+# ============================================================
+- name: test register domain identity check mode
+  block:
+    - name: register domain identity check mode
+      aws_ses_identity:
+        identity: "{{ domain_identity }}"
+        state: present
+        <<: *aws_connection_info
+      register: result
+      check_mode: True
+
+    - name: assert changed is True
+      assert:
+        that:
+          - result.changed == True
+
+    - import_tasks: assert_defaults.yaml
+      vars:
+        identity: "{{ domain_identity }}"
+
+  always:
+    - name: cleanup domain identity
+      aws_ses_identity:
+        identity: "{{ domain_identity }}"
+        state: absent
+        <<: *aws_connection_info
+      register: result
+
+    - name: assert nothing to clean up since check mode
+      assert:
+        that:
+          - result.changed == False
+# ============================================================
 - name: remove non-existent email identity
   aws_ses_identity:
     identity: "{{ email_identity }}"
@@ -133,6 +197,74 @@
     that:
       - result.changed == False
 # ============================================================
+- name: test remove email identity check mode
+  block:
+    - name: register email identity
+      aws_ses_identity:
+        identity: "{{ email_identity }}"
+        state: present
+        <<: *aws_connection_info
+      register: result
+
+    - name: remove email identity check mode
+      aws_ses_identity:
+        identity: "{{ email_identity }}"
+        state: absent
+        <<: *aws_connection_info
+      register: result
+      check_mode: True
+
+    - name: assert changed is True
+      assert:
+        that:
+          - result.changed == True
+  always:
+    - name: cleanup email identity
+      aws_ses_identity:
+        identity: "{{ email_identity }}"
+        state: absent
+        <<: *aws_connection_info
+      register: result
+
+    - name: assert something to clean up since remove was check mode
+      assert:
+        that:
+          - result.changed == True
+# ============================================================
+- name: test remove domain identity check mode
+  block:
+    - name: register domain identity
+      aws_ses_identity:
+        identity: "{{ domain_identity }}"
+        state: present
+        <<: *aws_connection_info
+      register: result
+
+    - name: remove domain identity check mode
+      aws_ses_identity:
+        identity: "{{ domain_identity }}"
+        state: absent
+        <<: *aws_connection_info
+      register: result
+      check_mode: True
+
+    - name: assert changed is True
+      assert:
+        that:
+          - result.changed == True
+  always:
+    - name: cleanup domain identity
+      aws_ses_identity:
+        identity: "{{ domain_identity }}"
+        state: absent
+        <<: *aws_connection_info
+      register: result
+
+    - name: assert something to clean up since remove was check mode
+      assert:
+        that:
+          - result.changed == True
+# ============================================================
 - name: test set notification queues
   block:
     - name: test topic
@@ -240,6 +372,95 @@
         state: absent
         <<: *aws_connection_info
 # ============================================================
+- name: test change notification settings check mode
+  block:
+    - name: test topic
+      sns_topic:
+        name: "{{ notification_queue_name }}-{{ item }}"
+        state: present
+        <<: *aws_connection_info
+      register: topic_info
+      with_items:
+        - bounce
+        - complaint
+        - delivery
+
+    - name: register email identity
+      aws_ses_identity:
+        identity: "{{ email_identity }}"
+        state: present
+        <<: *aws_connection_info
+
+    - name: set notification settings check mode
+      aws_ses_identity:
+        identity: "{{ email_identity }}"
+        state: present
+        bounce_notifications:
+          topic: "{{ topic_info.results[0].sns_arn }}"
+          include_headers: Yes
+        complaint_notifications:
+          topic: "{{ topic_info.results[1].sns_arn }}"
+          include_headers: Yes
+        delivery_notifications:
+          topic: "{{ topic_info.results[2].sns_arn }}"
+          include_headers: Yes
+        feedback_forwarding: No
+        <<: *aws_connection_info
+      register: result
+      check_mode: True
+
+    - name: assert changed is True
+      assert:
+        that:
+          - result.changed == True
+
+    - name: assert notification settings
+      assert:
+        that:
+          - result.notification_attributes.bounce_topic == topic_info.results[0].sns_arn
+          - result.notification_attributes.headers_in_bounce_notifications_enabled == True
+          - result.notification_attributes.delivery_topic == topic_info.results[2].sns_arn
+          - result.notification_attributes.headers_in_delivery_notifications_enabled == True
+          - result.notification_attributes.complaint_topic == topic_info.results[1].sns_arn
+          - result.notification_attributes.headers_in_complaint_notifications_enabled == True
+          - result.notification_attributes.forwarding_enabled == False
+
+    - name: re-register base email identity
+      aws_ses_identity:
+        identity: "{{ email_identity }}"
+        state: present
+        <<: *aws_connection_info
+      register: result
+
+    - name: assert no change since notifications were check mode
+      assert:
+        that:
+          - result.changed == False
+          - "'bounce_topic' not in result.notification_attributes"
+          - result.notification_attributes.headers_in_bounce_notifications_enabled == False
+          - "'delivery_topic' not in result.notification_attributes"
+          - result.notification_attributes.headers_in_delivery_notifications_enabled == False
+          - "'complaint_topic' not in result.notification_attributes"
+          - result.notification_attributes.headers_in_complaint_notifications_enabled == False
+          - result.notification_attributes.forwarding_enabled == True
+
+  always:
+    - name: cleanup topics
+      sns_topic:
+        name: "{{ notification_queue_name }}-{{ item }}"
+        state: absent
+        <<: *aws_connection_info
+      with_items:
+        - bounce
+        - complaint
+        - delivery
+
+    - name: cleanup email identity
+      aws_ses_identity:
+        identity: "{{ email_identity }}"
+        state: absent
+        <<: *aws_connection_info
+# ============================================================
 - name: test include headers on notification queues
   block:
     - name: register email identity