From c4536bc827e4b8faeb5faefc088159ac2bfe07d2 Mon Sep 17 00:00:00 2001 From: Ed Costello Date: Sat, 26 May 2018 10:46:25 +1200 Subject: [PATCH] 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. --- .../aws_ses_identity_check_mode.yaml | 2 + .../modules/cloud/amazon/aws_ses_identity.py | 199 ++++++++-------- .../targets/aws_ses_identity/tasks/main.yaml | 221 ++++++++++++++++++ 3 files changed, 323 insertions(+), 99 deletions(-) create mode 100644 changelogs/fragments/aws_ses_identity_check_mode.yaml diff --git a/changelogs/fragments/aws_ses_identity_check_mode.yaml b/changelogs/fragments/aws_ses_identity_check_mode.yaml new file mode 100644 index 0000000000..5fb1b8b92e --- /dev/null +++ b/changelogs/fragments/aws_ses_identity_check_mode.yaml @@ -0,0 +1,2 @@ +minor_changes: + - The aws_ses_identity module supports check mode diff --git a/lib/ansible/modules/cloud/amazon/aws_ses_identity.py b/lib/ansible/modules/cloud/amazon/aws_ses_identity.py index 5e869a76f9..b602f5b4f4 100644 --- a/lib/ansible/modules/cloud/amazon/aws_ses_identity.py +++ b/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.ec2 import camel_dict_to_snake_dict, ec2_argument_spec, get_aws_connection_info, boto3_conn -from ansible.module_utils.ec2 import HAS_BOTO3 +from ansible.module_utils.aws.core import AnsibleAWSModule +from ansible.module_utils.ec2 import camel_dict_to_snake_dict, AWSRetry 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( - module, - connection.set_identity_notification_topic, - Identity=identity, - NotificationType=notification_type, - SnsTopic=required, - ) + try: + if not module.check_mode: + connection.set_identity_notification_topic(Identity=identity, NotificationType=notification_type, SnsTopic=required, aws_retry=True) + except (BotoCoreError, ClientError) as e: + module.fail_json_aws(e, msg='Failed to set identity notification topic for {identity} {notification_type}'.format( + 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( - module, - connection.set_identity_headers_in_notifications_enabled, - Identity=identity, - NotificationType=notification_type, - Enabled=required, - ) + try: + if not module.check_mode: + connection.set_identity_headers_in_notifications_enabled(Identity=identity, NotificationType=notification_type, Enabled=required, + aws_retry=True) + except (BotoCoreError, ClientError) as e: + 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( - module, - connection.set_identity_feedback_forwarding_enabled, - Identity=identity, - ForwardingEnabled=required, - ) + try: + if not module.check_mode: + connection.set_identity_feedback_forwarding_enabled(Identity=identity, ForwardingEnabled=required, aws_retry=True) + except (BotoCoreError, ClientError) as e: + 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: - call_and_handle_errors(module, connection.verify_email_identity, EmailAddress=identity) + try: + 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): - caller_identity = sts.get_caller_identity() +def get_account_id(module): + 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() - - argument_spec.update( - dict( - identity=dict(required=True, type='str'), - state=dict(default='present', choices=['present', 'absent']), - bounce_notifications=dict(type='dict'), - complaint_notifications=dict(type='dict'), - delivery_notifications=dict(type='dict'), - feedback_forwarding=dict(default=True, type='bool'), - ) + module = AnsibleAWSModule( + argument_spec={ + "identity": dict(required=True, type='str'), + "state": dict(default='present', choices=['present', 'absent']), + "bounce_notifications": dict(type='dict'), + "complaint_notifications": dict(type='dict'), + "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. - # As a result there are build failures due to throttling that exceeds boto's default retries. - # The back-off is exponential, so upping the retry attempts allows multiple parallel runs - # 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) + # the ansible build runs multiple instances of the test in parallel that's caused throttling + # failures so apply a jittered backoff to call SES calls. + connection = module.client('ses', retry_decorator=AWSRetry.jittered_backoff()) 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) - account_id = get_account_id(sts) + region = module.params.get('region') + 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) diff --git a/test/integration/targets/aws_ses_identity/tasks/main.yaml b/test/integration/targets/aws_ses_identity/tasks/main.yaml index e860a7baba..9775331602 100644 --- a/test/integration/targets/aws_ses_identity/tasks/main.yaml +++ b/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