diff --git a/lib/ansible/modules/cloud/amazon/aws_ses_identity.py b/lib/ansible/modules/cloud/amazon/aws_ses_identity.py index 991ccd4cb6..9ce1a73309 100644 --- a/lib/ansible/modules/cloud/amazon/aws_ses_identity.py +++ b/lib/ansible/modules/cloud/amazon/aws_ses_identity.py @@ -235,10 +235,13 @@ 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 +import time + import traceback try: - from botocore.exceptions import BotoCoreError, ClientError, ParamValidationError + from botocore.exceptions import BotoCoreError, ClientError + from botocore.config import Config except ImportError: pass # caught by imported HAS_BOTO3 @@ -253,17 +256,54 @@ def call_and_handle_errors(module, method, **kwargs): module.fail_json(msg=str(e), exception=traceback.format_exc()) -def get_verification_attributes(connection, module, identity): - response = call_and_handle_errors(module, connection.get_identity_verification_attributes, Identities=[identity]) - identity_verification = response['VerificationAttributes'] +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. + # Don't want this complexity exposed users of the module as they'd have to retry to ensure + # 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]) + identity_verification = response['VerificationAttributes'] + if identity in identity_verification: + break + time.sleep(retryDelay) if identity not in identity_verification: return None return identity_verification[identity] -def get_identity_notifications(connection, module, identity): - response = call_and_handle_errors(module, connection.get_identity_notification_attributes, Identities=[identity]) - notification_attributes = response['NotificationAttributes'] +def get_identity_notifications(connection, module, identity, retries=0, retryDelay=10): + # Unpredictably get_identity_notifications doesn't include the notifications when we've + # just registered the identity. + # Don't want this complexity exposed users of the module as they'd have to retry to ensure + # a consistent return from the module. + # 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]) + notification_attributes = response['NotificationAttributes'] + + # No clear AWS docs on when this happens, but it appears sometimes identities are not included in + # in the notification attributes when the identity is first registered. Suspect that this is caused by + # eventual consistency within the AWS services. It's been observed in builds so we need to handle it. + # + # When this occurs, just return None and we'll assume no identity notification settings have been changed + # from the default which is reasonable if this is just eventual consistency on creation. + # See: https://github.com/ansible/ansible/issues/36065 + if identity in notification_attributes: + break + else: + # Paranoia check for coding errors, we only requested one identity, so if we get a different one + # something has gone very wrong. + if len(notification_attributes) != 0: + module.fail_json( + msg='Unexpected identity found in notification attributes, expected {0} but got {1!r}.'.format( + identity, + notification_attributes.keys(), + ) + ) + time.sleep(retryDelay) if identity not in notification_attributes: return None return notification_attributes[identity] @@ -272,9 +312,17 @@ def get_identity_notifications(connection, module, identity): def update_notification_topic(connection, module, identity, identity_notifications, notification_type): arg_dict = module.params.get(notification_type.lower() + '_notifications') topic_key = notification_type + 'Topic' - if topic_key in identity_notifications: + if identity_notifications is None: + # If there is no configuration for notifications cannot be being sent to topics + # hence assume None as the current state. + current = None + elif topic_key in identity_notifications: current = identity_notifications[topic_key] else: + # If there is information on the notifications setup but no information on the + # particular notification topic it's pretty safe to assume there's no topic for + # this notification. AWS API docs suggest this information will always be + # included but best to be defensive current = None if arg_dict is not None and 'topic' in arg_dict: @@ -297,9 +345,16 @@ def update_notification_topic(connection, module, identity, identity_notificatio def update_notification_topic_headers(connection, module, identity, identity_notifications, notification_type): arg_dict = module.params.get(notification_type.lower() + '_notifications') header_key = 'HeadersIn' + notification_type + 'NotificationsEnabled' - if header_key in identity_notifications: + if identity_notifications is None: + # If there is no configuration for topic notifications, headers cannot be being + # forwarded, hence assume false. + current = False + elif header_key in identity_notifications: current = identity_notifications[header_key] else: + # AWS API doc indicates that the headers in fields are optional. Unfortunately + # it's not clear on what this means. But it's a pretty safe assumption that it means + # headers are not included since most API consumers would interpret absence as false. current = False if arg_dict is not None and 'include_headers' in arg_dict: @@ -320,9 +375,17 @@ def update_notification_topic_headers(connection, module, identity, identity_not def update_feedback_forwarding(connection, module, identity, identity_notifications): - if 'ForwardingEnabled' in identity_notifications: + 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 + # information existing feedback forwarding must be on. + current = True + elif 'ForwardingEnabled' in identity_notifications: current = identity_notifications['ForwardingEnabled'] else: + # If there is information on the notifications setup but no information on the + # forwarding state it's pretty safe to assume forwarding is off. AWS API docs + # suggest this information will always be included but best to be defensive current = False required = module.params.get('feedback_forwarding') @@ -349,8 +412,8 @@ def update_identity_notifications(connection, module): changed |= update_feedback_forwarding(connection, module, identity, identity_notifications) - if changed: - identity_notifications = get_identity_notifications(connection, module, identity) + if changed or identity_notifications is None: + identity_notifications = get_identity_notifications(connection, module, identity, retries=4) return changed, identity_notifications @@ -363,15 +426,21 @@ def create_or_update_identity(connection, module, region, account_id): call_and_handle_errors(module, connection.verify_email_identity, EmailAddress=identity) else: call_and_handle_errors(module, connection.verify_domain_identity, Domain=identity) - verification_attributes = get_verification_attributes(connection, module, identity) + 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'], verification_attributes=camel_dict_to_snake_dict(verification_attributes)) + if verification_attributes is None: + module.fail_json(msg='Unable to load identity verification attributes after registering identity.') + notifications_changed, notification_attributes = update_identity_notifications(connection, module) changed |= notifications_changed + if notification_attributes is None: + module.fail_json(msg='Unable to load identity notification attributes.') + identity_arn = 'arn:aws:ses:' + region + ':' + account_id + ':identity/' + identity module.exit_json( @@ -433,7 +502,15 @@ def main(): region, ec2_url, aws_connect_params = get_aws_connection_info(module, boto3=True) - connection = boto3_conn(module, conn_type='client', resource='ses', region=region, endpoint=ec2_url, **aws_connect_params) + # 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) state = module.params.get("state") @@ -444,5 +521,6 @@ def main(): else: destroy_identity(connection, module) + if __name__ == '__main__': main()