From 0164b48e96e3785d7c4783f438ced544e45d800b Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Thu, 20 May 2021 20:37:56 +0200 Subject: [PATCH] Cloudformation: use is_boto3_error_message (#355) Cloudformation: use is_boto3_error_message Reviewed-by: https://github.com/apps/ansible-zuul --- plugins/modules/cloudformation.py | 62 +++++++++---------- .../plugins/modules/test_cloudformation.py | 34 +++++++--- 2 files changed, 52 insertions(+), 44 deletions(-) diff --git a/plugins/modules/cloudformation.py b/plugins/modules/cloudformation.py index fd42caeac52..e4eb99e52e1 100644 --- a/plugins/modules/cloudformation.py +++ b/plugins/modules/cloudformation.py @@ -342,6 +342,7 @@ from ansible.module_utils._text import to_native from ..module_utils.core import AnsibleAWSModule +from ..module_utils.core import is_boto3_error_message from ..module_utils.ec2 import AWSRetry from ..module_utils.ec2 import ansible_dict_to_boto3_tag_list from ..module_utils.ec2 import boto_exception @@ -364,12 +365,11 @@ def get_stack_events(cfn, stack_name, events_limit, token_filter=None): )) else: events = list(pg.search("StackEvents[*]")) - except (botocore.exceptions.ValidationError, botocore.exceptions.ClientError) as err: + except is_boto3_error_message('does not exist'): + ret['log'].append('Stack does not exist.') + return ret + except (botocore.exceptions.ValidationError, botocore.exceptions.ClientError) as err: # pylint: disable=duplicate-except error_msg = boto_exception(err) - if 'does not exist' in error_msg: - # missing stack, don't bail. - ret['log'].append('Stack does not exist.') - return ret ret['log'].append('Unknown error: ' + str(error_msg)) return ret @@ -406,7 +406,7 @@ def create_stack(module, stack_params, cfn, events_limit): try: response = cfn.create_stack(**stack_params) # Use stack ID to follow stack state in case of on_create_failure = DELETE - result = stack_operation(cfn, response['StackId'], 'CREATE', events_limit, stack_params.get('ClientRequestToken', None)) + result = stack_operation(module, cfn, response['StackId'], 'CREATE', events_limit, stack_params.get('ClientRequestToken', None)) except Exception as err: module.fail_json_aws(err, msg="Failed to create stack {0}".format(stack_params.get('StackName'))) if not result: @@ -459,17 +459,15 @@ def create_changeset(module, stack_params, cfn, events_limit): break # Lets not hog the cpu/spam the AWS API time.sleep(1) - result = stack_operation(cfn, stack_params['StackName'], 'CREATE_CHANGESET', events_limit) + result = stack_operation(module, cfn, stack_params['StackName'], 'CREATE_CHANGESET', events_limit) result['change_set_id'] = cs['Id'] result['warnings'] = ['Created changeset named %s for stack %s' % (changeset_name, stack_params['StackName']), 'You can execute it using: aws cloudformation execute-change-set --change-set-name %s' % cs['Id'], 'NOTE that dependencies on this stack might fail due to pending changes!'] + except is_boto3_error_message('No updates are to be performed.'): + result = dict(changed=False, output='Stack is already up-to-date.') except Exception as err: - error_msg = boto_exception(err) - if 'No updates are to be performed.' in error_msg: - result = dict(changed=False, output='Stack is already up-to-date.') - else: - module.fail_json_aws(err, msg='Failed to create change set') + module.fail_json_aws(err, msg='Failed to create change set') if not result: module.fail_json(msg="empty result") @@ -488,13 +486,11 @@ def update_stack(module, stack_params, cfn, events_limit): # don't need to be updated. try: cfn.update_stack(**stack_params) - result = stack_operation(cfn, stack_params['StackName'], 'UPDATE', events_limit, stack_params.get('ClientRequestToken', None)) + result = stack_operation(module, cfn, stack_params['StackName'], 'UPDATE', events_limit, stack_params.get('ClientRequestToken', None)) + except is_boto3_error_message('No updates are to be performed.'): + result = dict(changed=False, output='Stack is already up-to-date.') except Exception as err: - error_msg = boto_exception(err) - if 'No updates are to be performed.' in error_msg: - result = dict(changed=False, output='Stack is already up-to-date.') - else: - module.fail_json_aws(err, msg="Failed to update stack {0}".format(stack_params.get('StackName'))) + module.fail_json_aws(err, msg="Failed to update stack {0}".format(stack_params.get('StackName'))) if not result: module.fail_json(msg="empty result") return result @@ -504,7 +500,7 @@ def update_termination_protection(module, cfn, stack_name, desired_termination_p '''updates termination protection of a stack''' if not boto_supports_termination_protection(cfn): module.fail_json(msg="termination_protection parameter requires botocore >= 1.7.18") - stack = get_stack_facts(cfn, stack_name) + stack = get_stack_facts(module, cfn, stack_name) if stack: if stack['EnableTerminationProtection'] is not desired_termination_protection_state: try: @@ -520,12 +516,12 @@ def boto_supports_termination_protection(cfn): return hasattr(cfn, "update_termination_protection") -def stack_operation(cfn, stack_name, operation, events_limit, op_token=None): +def stack_operation(module, cfn, stack_name, operation, events_limit, op_token=None): '''gets the status of a stack while it is created/updated/deleted''' existed = [] while True: try: - stack = get_stack_facts(cfn, stack_name) + stack = get_stack_facts(module, cfn, stack_name, raise_errors=True) existed.append('yes') except Exception: # If the stack previously existed, and now can't be found then it's @@ -611,18 +607,16 @@ def check_mode_changeset(module, stack_params, cfn): module.fail_json_aws(err) -def get_stack_facts(cfn, stack_name): +def get_stack_facts(module, cfn, stack_name, raise_errors=False): try: stack_response = cfn.describe_stacks(StackName=stack_name) stack_info = stack_response['Stacks'][0] - except (botocore.exceptions.ValidationError, botocore.exceptions.ClientError) as err: - error_msg = boto_exception(err) - if 'does not exist' in error_msg: - # missing stack, don't bail. - return None - - # other error, bail. - raise err + except is_boto3_error_message('does not exist'): + return None + except (botocore.exceptions.ValidationError, botocore.exceptions.ClientError) as err: # pylint: disable=duplicate-except + if raise_errors: + raise err + module.fail_json_aws(err, msg="Failed to describe stack") if stack_response and stack_response.get('Stacks', None): stacks = stack_response['Stacks'] @@ -753,7 +747,7 @@ def main(): if boto_supports_termination_protection(cfn): cfn.update_termination_protection = backoff_wrapper(cfn.update_termination_protection) - stack_info = get_stack_facts(cfn, stack_params['StackName']) + stack_info = get_stack_facts(module, cfn, stack_params['StackName']) if module.check_mode: if state == 'absent' and stack_info: @@ -778,7 +772,7 @@ def main(): # format the stack output - stack = get_stack_facts(cfn, stack_params['StackName']) + stack = get_stack_facts(module, cfn, stack_params['StackName']) if stack is not None: if result.get('stack_outputs') is None: # always define stack_outputs, but it may be empty @@ -804,7 +798,7 @@ def main(): # so must describe the stack first try: - stack = get_stack_facts(cfn, stack_params['StackName']) + stack = get_stack_facts(module, cfn, stack_params['StackName']) if not stack: result = {'changed': False, 'output': 'Stack not found.'} else: @@ -812,7 +806,7 @@ def main(): cfn.delete_stack(StackName=stack_params['StackName']) else: cfn.delete_stack(StackName=stack_params['StackName'], RoleARN=stack_params['RoleARN']) - result = stack_operation(cfn, stack_params['StackName'], 'DELETE', module.params.get('events_limit'), + result = stack_operation(module, cfn, stack_params['StackName'], 'DELETE', module.params.get('events_limit'), stack_params.get('ClientRequestToken', None)) except Exception as err: module.fail_json_aws(err) diff --git a/tests/unit/plugins/modules/test_cloudformation.py b/tests/unit/plugins/modules/test_cloudformation.py index 6ee1fcf95d5..3b0e7c9fb5e 100644 --- a/tests/unit/plugins/modules/test_cloudformation.py +++ b/tests/unit/plugins/modules/test_cloudformation.py @@ -12,6 +12,9 @@ # Magic... from ansible_collections.amazon.aws.tests.unit.utils.amazon_placebo_fixtures import maybe_sleep, placeboify # pylint: disable=unused-import +import ansible_collections.amazon.aws.plugins.module_utils.core as aws_core +import ansible_collections.amazon.aws.plugins.module_utils.ec2 as aws_ec2 + from ansible_collections.amazon.aws.plugins.module_utils.ec2 import boto_exception from ansible_collections.amazon.aws.plugins.modules import cloudformation as cfn_module @@ -78,8 +81,15 @@ def exit_json(self, *args, **kwargs): raise Exception('EXIT') -def test_invalid_template_json(placeboify): +def _create_wrapped_client(placeboify): connection = placeboify.client('cloudformation') + retry_decorator = aws_ec2.AWSRetry.jittered_backoff() + wrapped_conn = aws_core._RetryingBotoClientWrapper(connection, retry_decorator) + return wrapped_conn + + +def test_invalid_template_json(placeboify): + connection = _create_wrapped_client(placeboify) params = { 'StackName': 'ansible-test-wrong-json', 'TemplateBody': bad_json_tpl, @@ -94,7 +104,7 @@ def test_invalid_template_json(placeboify): def test_client_request_token_s3_stack(maybe_sleep, placeboify): - connection = placeboify.client('cloudformation') + connection = _create_wrapped_client(placeboify) params = { 'StackName': 'ansible-test-client-request-token-yaml', 'TemplateBody': basic_yaml_tpl, @@ -111,7 +121,7 @@ def test_client_request_token_s3_stack(maybe_sleep, placeboify): def test_basic_s3_stack(maybe_sleep, placeboify): - connection = placeboify.client('cloudformation') + connection = _create_wrapped_client(placeboify) params = { 'StackName': 'ansible-test-basic-yaml', 'TemplateBody': basic_yaml_tpl @@ -127,15 +137,19 @@ def test_basic_s3_stack(maybe_sleep, placeboify): def test_delete_nonexistent_stack(maybe_sleep, placeboify): - connection = placeboify.client('cloudformation') - result = cfn_module.stack_operation(connection, 'ansible-test-nonexist', 'DELETE', default_events_limit) + connection = _create_wrapped_client(placeboify) + # module is only used if we threw an unexpected error + module = None + result = cfn_module.stack_operation(module, connection, 'ansible-test-nonexist', 'DELETE', default_events_limit) assert result['changed'] assert 'Stack does not exist.' in result['log'] def test_get_nonexistent_stack(placeboify): - connection = placeboify.client('cloudformation') - assert cfn_module.get_stack_facts(connection, 'ansible-test-nonexist') is None + connection = _create_wrapped_client(placeboify) + # module is only used if we threw an unexpected error + module = None + assert cfn_module.get_stack_facts(module, connection, 'ansible-test-nonexist') is None def test_missing_template_body(): @@ -159,7 +173,7 @@ def test_on_create_failure_delete(maybe_sleep, placeboify): on_create_failure='DELETE', disable_rollback=False, ) - connection = placeboify.client('cloudformation') + connection = _create_wrapped_client(placeboify) params = { 'StackName': 'ansible-test-on-create-failure-delete', 'TemplateBody': failing_yaml_tpl @@ -178,7 +192,7 @@ def test_on_create_failure_rollback(maybe_sleep, placeboify): on_create_failure='ROLLBACK', disable_rollback=False, ) - connection = placeboify.client('cloudformation') + connection = _create_wrapped_client(placeboify) params = { 'StackName': 'ansible-test-on-create-failure-rollback', 'TemplateBody': failing_yaml_tpl @@ -198,7 +212,7 @@ def test_on_create_failure_do_nothing(maybe_sleep, placeboify): on_create_failure='DO_NOTHING', disable_rollback=False, ) - connection = placeboify.client('cloudformation') + connection = _create_wrapped_client(placeboify) params = { 'StackName': 'ansible-test-on-create-failure-do-nothing', 'TemplateBody': failing_yaml_tpl