diff --git a/changelogs/fragments/20240107-refactor_ec2_ami-modules.yml b/changelogs/fragments/20240107-refactor_ec2_ami-modules.yml new file mode 100644 index 00000000000..14c8bc4da3a --- /dev/null +++ b/changelogs/fragments/20240107-refactor_ec2_ami-modules.yml @@ -0,0 +1,4 @@ +--- +minor_changes: + - ec2_ami - refactored code to use ``AnsibleEC2Error`` as well as moving shared code into module_utils.ec2 (https://github.com/ansible-collections/amazon.aws/pull/2164). + - ec2_ami_info - refactored code to use ``AnsibleEC2Error`` as well as moving shared code into module_utils.ec2 (https://github.com/ansible-collections/amazon.aws/pull/2164). diff --git a/plugins/module_utils/ec2.py b/plugins/module_utils/ec2.py index 9e8befe618d..9adf6fdfc44 100644 --- a/plugins/module_utils/ec2.py +++ b/plugins/module_utils/ec2.py @@ -792,8 +792,12 @@ def _is_missing(cls): def describe_images( client, **params: Dict[str, Union[List[str], bool, int, List[Dict[str, Union[str, List[str]]]]]] ) -> List[Dict[str, Any]]: - paginator = client.get_paginator("describe_images") - return paginator.paginate(**params).build_full_result()["Images"] + # 'DescribeImages' can be paginated depending on the boto3 version + if client.can_paginate("describe_images"): + paginator = client.get_paginator("describe_images") + return paginator.paginate(**params).build_full_result()["Images"] + else: + return client.describe_images(**params)["Images"] @EC2ImageErrorHandler.list_error_handler("describe image attribute", {}) diff --git a/plugins/modules/ec2_ami.py b/plugins/modules/ec2_ami.py index 699f22680c3..1ccd02e82c6 100644 --- a/plugins/modules/ec2_ami.py +++ b/plugins/modules/ec2_ami.py @@ -461,21 +461,22 @@ import time -try: - import botocore -except ImportError: - pass # Handled by AnsibleAWSModule - from ansible.module_utils.common.dict_transformations import camel_dict_to_snake_dict -from ansible_collections.amazon.aws.plugins.module_utils.botocore import is_boto3_error_code +from ansible_collections.amazon.aws.plugins.module_utils.ec2 import AnsibleEC2Error from ansible_collections.amazon.aws.plugins.module_utils.ec2 import add_ec2_tags +from ansible_collections.amazon.aws.plugins.module_utils.ec2 import create_image +from ansible_collections.amazon.aws.plugins.module_utils.ec2 import delete_snapshot +from ansible_collections.amazon.aws.plugins.module_utils.ec2 import deregister_image +from ansible_collections.amazon.aws.plugins.module_utils.ec2 import describe_image_attribute +from ansible_collections.amazon.aws.plugins.module_utils.ec2 import describe_images from ansible_collections.amazon.aws.plugins.module_utils.ec2 import ensure_ec2_tags +from ansible_collections.amazon.aws.plugins.module_utils.ec2 import modify_image_attribute +from ansible_collections.amazon.aws.plugins.module_utils.ec2 import register_image from ansible_collections.amazon.aws.plugins.module_utils.modules import AnsibleAWSModule -from ansible_collections.amazon.aws.plugins.module_utils.retries import AWSRetry from ansible_collections.amazon.aws.plugins.module_utils.tagging import boto3_tag_list_to_ansible_dict from ansible_collections.amazon.aws.plugins.module_utils.tagging import boto3_tag_specifications -from ansible_collections.amazon.aws.plugins.module_utils.waiters import get_waiter +from ansible_collections.amazon.aws.plugins.module_utils.waiters import wait_for_resource_state class Ec2AmiFailure(Exception): @@ -539,32 +540,25 @@ def get_ami_info(camel_image): def get_image_by_id(connection, image_id): try: - images_response = connection.describe_images(aws_retry=True, ImageIds=[image_id]) - except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e: + images = describe_images(connection, ImageIds=[image_id]) + except AnsibleEC2Error as e: raise Ec2AmiFailure("Error retrieving image by image_id", e) - images = images_response.get("Images", []) - image_counter = len(images) - if image_counter == 0: + if len(images) == 0: return None - if image_counter > 1: + if len(images) > 1: raise Ec2AmiFailure(f"Invalid number of instances ({str(len(images))}) found for image_id: {image_id}.") result = images[0] try: - result["LaunchPermissions"] = connection.describe_image_attribute( - aws_retry=True, Attribute="launchPermission", ImageId=image_id - )["LaunchPermissions"] - result["ProductCodes"] = connection.describe_image_attribute( - aws_retry=True, Attribute="productCodes", ImageId=image_id - )["ProductCodes"] - except is_boto3_error_code("InvalidAMIID.Unavailable"): - pass - except ( - botocore.exceptions.BotoCoreError, - botocore.exceptions.ClientError, - ) as e: # pylint: disable=duplicate-except + image_attribue = describe_image_attribute(connection, attribute="launchPermission", image_id=image_id) + if image_attribue: + result["LaunchPermissions"] = image_attribue["LaunchPermissions"] + image_attribue = describe_image_attribute(connection, attribute="productCodes", image_id=image_id) + if image_attribue: + result["ProductCodes"] = image_attribue["ProductCodes"] + except AnsibleEC2Error as e: raise Ec2AmiFailure(f"Error retrieving image attributes for image {image_id}", e) return result @@ -629,14 +623,9 @@ def purge_snapshots(connection): snapshot_id = mapping.get("Ebs", {}).get("SnapshotId") if snapshot_id is None: continue - connection.delete_snapshot(aws_retry=True, SnapshotId=snapshot_id) + delete_snapshot(connection, snapshot_id) yield snapshot_id - except is_boto3_error_code("InvalidSnapshot.NotFound"): - pass - except ( - botocore.exceptions.ClientError, - botocore.exceptions.BotoCoreError, - ) as e: # pylint: disable=duplicate-except + except AnsibleEC2Error as e: raise Ec2AmiFailure("Failed to delete snapshot.", e) return purge_snapshots @@ -670,8 +659,8 @@ def do(cls, module, connection, image_id): # When trying to re-deregister an already deregistered image it doesn't raise an exception, it just returns an object without image attributes. if "ImageId" in image: try: - connection.deregister_image(aws_retry=True, ImageId=image_id) - except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e: + deregister_image(connection, image_id) + except AnsibleEC2Error as e: raise Ec2AmiFailure("Error deregistering image", e) else: module.exit_json(msg=f"Image {image_id} has already been deregistered.", changed=False) @@ -738,14 +727,14 @@ def set_launch_permission(connection, image, launch_permissions, check_mode): try: if not check_mode: - connection.modify_image_attribute( - aws_retry=True, - ImageId=image["ImageId"], + changed = modify_image_attribute( + connection, + image_id=image["ImageId"], Attribute="launchPermission", LaunchPermission=dict(Add=to_add, Remove=to_remove), ) changed = True - except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e: + except AnsibleEC2Error as e: raise Ec2AmiFailure(f"Error updating launch permissions of image {image['ImageId']}", e) return changed @@ -766,14 +755,14 @@ def set_description(connection, module, image, description): try: if not module.check_mode: - connection.modify_image_attribute( - aws_retry=True, + modify_image_attribute( + connection, + image_id=image["ImageId"], Attribute="Description", - ImageId=image["ImageId"], Description={"Value": description}, ) return True - except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e: + except AnsibleEC2Error as e: raise Ec2AmiFailure(f"Error setting description for image {image['ImageId']}", e) @classmethod @@ -804,21 +793,20 @@ def do(cls, module, connection, image_id): class CreateImage: @staticmethod def do_check_mode(module, connection, _image_id): - image = connection.describe_images(Filters=[{"Name": "name", "Values": [str(module.params["name"])]}]) - if not image["Images"]: + images = describe_images(connection, Filters=[{"Name": "name", "Values": [str(module.params["name"])]}]) + if not images: module.exit_json(changed=True, msg="Would have created a AMI if not in check mode.") else: module.exit_json(changed=False, msg="Error registering image: AMI name is already in use by another AMI") @staticmethod - def wait(connection, wait_timeout, image_id): - if not wait_timeout: - return - - delay = 15 - max_attempts = wait_timeout // delay - waiter = get_waiter(connection, "image_available") - waiter.wait(ImageIds=[image_id], WaiterConfig={"Delay": delay, "MaxAttempts": max_attempts}) + def wait(connection, module, image_id): + if module.params.get("wait") and module.params.get("wait_timeout"): + delay = 15 + max_attempts = module.params.get("wait_timeout") // delay + wait_for_resource_state( + connection, module, "image_available", delay=delay, max_attempts=max_attempts, ImageIds=[image_id] + ) @staticmethod def set_tags(connection, module, tags, image_id): @@ -841,7 +829,7 @@ def set_launch_permissions(connection, launch_permissions, image_id): # remove any keys with value=None launch_permissions = {k: v for k, v in launch_permissions.items() if v is not None} try: - params = {"Attribute": "LaunchPermission", "ImageId": image_id, "LaunchPermission": {"Add": []}} + params = {"Attribute": "LaunchPermission", "LaunchPermission": {"Add": []}} for group_name in launch_permissions.get("group_names", []): params["LaunchPermission"]["Add"].append(dict(Group=group_name)) for user_id in launch_permissions.get("user_ids", []): @@ -851,14 +839,14 @@ def set_launch_permissions(connection, launch_permissions, image_id): for org_unit_arn in launch_permissions.get("org_unit_arns", []): params["LaunchPermission"]["Add"].append(dict(OrganizationalUnitArn=org_unit_arn)) if params["LaunchPermission"]["Add"]: - connection.modify_image_attribute(aws_retry=True, **params) - except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e: + modify_image_attribute(connection, image_id=image_id, **params) + except AnsibleEC2Error as e: raise Ec2AmiFailure(f"Error setting launch permissions for image {image_id}", e) @staticmethod - def create_or_register(connection, create_image_parameters): + def create_or_register(create_image_parameters): create_from_instance = "InstanceId" in create_image_parameters - func = connection.create_image if create_from_instance else connection.register_image + func = create_image if create_from_instance else register_image return func @staticmethod @@ -950,14 +938,14 @@ def do(cls, module, connection, _image_id): """Entry point to create image""" create_image_parameters = cls.build_create_image_parameters(**module.params) - func = cls.create_or_register(connection, create_image_parameters) + func = cls.create_or_register(create_image_parameters) try: - image = func(aws_retry=True, **create_image_parameters) + image = func(connection, **create_image_parameters) image_id = image.get("ImageId") - except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e: - raise Ec2AmiFailure("Error registering image", e) + except AnsibleEC2Error as e: + raise Ec2AmiFailure("Error registering/creating image", e) - cls.wait(connection, module.params.get("wait") and module.params.get("wait_timeout"), image_id) + cls.wait(connection, module, image_id) if "TagSpecifications" not in create_image_parameters: CreateImage.set_tags(connection, module, module.params.get("tags"), image_id) @@ -1027,7 +1015,7 @@ def main(): validate_params(module, **module.params) - connection = module.client("ec2", retry_decorator=AWSRetry.jittered_backoff()) + connection = module.client("ec2") CHECK_MODE_TRUE = True CHECK_MODE_FALSE = False diff --git a/plugins/modules/ec2_ami_info.py b/plugins/modules/ec2_ami_info.py index fb6a7cc2d93..92e59679a06 100644 --- a/plugins/modules/ec2_ami_info.py +++ b/plugins/modules/ec2_ami_info.py @@ -207,17 +207,14 @@ sample: hvm """ -try: - from botocore.exceptions import BotoCoreError - from botocore.exceptions import ClientError -except ImportError: - pass # Handled by AnsibleAWSModule - from ansible.module_utils.common.dict_transformations import camel_dict_to_snake_dict -from ansible_collections.amazon.aws.plugins.module_utils.botocore import is_boto3_error_code +from ansible_collections.amazon.aws.plugins.module_utils.ec2 import AnsibleEC2Error +from ansible_collections.amazon.aws.plugins.module_utils.ec2 import describe_image_attribute +from ansible_collections.amazon.aws.plugins.module_utils.ec2 import describe_images +from ansible_collections.amazon.aws.plugins.module_utils.exceptions import AnsibleAWSError +from ansible_collections.amazon.aws.plugins.module_utils.exceptions import is_ansible_aws_error_code from ansible_collections.amazon.aws.plugins.module_utils.modules import AnsibleAWSModule -from ansible_collections.amazon.aws.plugins.module_utils.retries import AWSRetry from ansible_collections.amazon.aws.plugins.module_utils.tagging import boto3_tag_list_to_ansible_dict from ansible_collections.amazon.aws.plugins.module_utils.transformation import ansible_dict_to_boto3_filter_list @@ -262,38 +259,29 @@ def build_request_args(executable_users, filters, image_ids, owners): def get_images(ec2_client, request_args): try: - images = ec2_client.describe_images(aws_retry=True, **request_args) - except (ClientError, BotoCoreError) as err: - raise AmiInfoFailure(err, "error describing images") + images = describe_images(ec2_client, **request_args) + except AnsibleEC2Error as e: + raise AmiInfoFailure(e, "error describing images") return images -def get_image_attribute(ec2_client, image_id): - try: - launch_permissions = ec2_client.describe_image_attribute( - aws_retry=True, Attribute="launchPermission", ImageId=image_id - ) - except (ClientError, BotoCoreError) as err: - raise AmiInfoFailure(err, "error describing image attribute") - return launch_permissions - - def list_ec2_images(ec2_client, module, request_args): - images = get_images(ec2_client, request_args)["Images"] + images = get_images(ec2_client, request_args) images = [camel_dict_to_snake_dict(image) for image in images] for image in images: try: - image_id = image["image_id"] image["tags"] = boto3_tag_list_to_ansible_dict(image.get("tags", [])) if module.params.get("describe_image_attributes"): - launch_permissions = get_image_attribute(ec2_client, image_id).get("LaunchPermissions", []) + launch_permissions = describe_image_attribute( + ec2_client, attribute="launchPermission", image_id=image["image_id"] + ).get("LaunchPermissions", []) image["launch_permissions"] = [camel_dict_to_snake_dict(perm) for perm in launch_permissions] - except is_boto3_error_code("AuthFailure"): + except is_ansible_aws_error_code("AuthFailure"): # describing launch permissions of images owned by others is not permitted, but shouldn't cause failures pass - except (ClientError, BotoCoreError) as err: # pylint: disable=duplicate-except - raise AmiInfoFailure(err, "Failed to describe AMI") + except AnsibleAWSError as e: # pylint: disable=duplicate-except + raise AmiInfoFailure(e, "Failed to describe AMI") images.sort(key=lambda e: e.get("creation_date", "")) # it may be possible that creation_date does not always exist @@ -311,7 +299,7 @@ def main(): module = AnsibleAWSModule(argument_spec=argument_spec, supports_check_mode=True) - ec2_client = module.client("ec2", retry_decorator=AWSRetry.jittered_backoff()) + ec2_client = module.client("ec2") request_args = build_request_args( executable_users=module.params["executable_users"], diff --git a/tests/unit/plugins/modules/test_ec2_ami.py b/tests/unit/plugins/modules/test_ec2_ami.py index b1e23451ba9..dfec225e240 100644 --- a/tests/unit/plugins/modules/test_ec2_ami.py +++ b/tests/unit/plugins/modules/test_ec2_ami.py @@ -1,6 +1,7 @@ # This file is part of Ansible # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) +from unittest.mock import ANY from unittest.mock import MagicMock from unittest.mock import call from unittest.mock import patch @@ -12,8 +13,9 @@ module_name = "ansible_collections.amazon.aws.plugins.modules.ec2_ami" +@patch(module_name + ".register_image") @patch(module_name + ".get_image_by_id") -def test_create_image_uefi_data(m_get_image_by_id): +def test_create_image_uefi_data(m_get_image_by_id, m_register_image): module = MagicMock() connection = MagicMock() @@ -31,11 +33,11 @@ def test_create_image_uefi_data(m_get_image_by_id): } ec2_ami.CreateImage.do(module, connection, None) - assert connection.register_image.call_count == 1 - connection.register_image.assert_has_calls( + assert m_register_image.call_count == 1 + m_register_image.assert_has_calls( [ call( - aws_retry=True, + connection, Name="my-image", BootMode="uefi", TpmSupport="v2.0", @@ -51,61 +53,67 @@ def test_get_block_device_mapping_virtual_name(): assert block_device == {"/dev/sdc": {"virtual_name": "ephemeral0"}} -def test_get_image_by_id_found(): +@patch(module_name + ".describe_image_attribute") +@patch(module_name + ".describe_images") +def test_get_image_by_id_found(m_describe_images, m_describe_image_attribute): connection = MagicMock() - connection.describe_images.return_value = {"Images": [{"ImageId": "ami-0c7a795306730b288"}]} + m_describe_images.return_value = [{"ImageId": "ami-0c7a795306730b288"}] image = ec2_ami.get_image_by_id(connection, "ami-0c7a795306730b288") assert image["ImageId"] == "ami-0c7a795306730b288" - assert connection.describe_images.call_count == 1 - assert connection.describe_image_attribute.call_count == 2 - connection.describe_images.assert_has_calls( + assert m_describe_images.call_count == 1 + assert m_describe_image_attribute.call_count == 2 + m_describe_images.assert_has_calls( [ call( - aws_retry=True, + connection, ImageIds=["ami-0c7a795306730b288"], ) ] ) -def test_get_image_by_too_many(): +@patch(module_name + ".describe_images") +def test_get_image_by_too_many(m_describe_images): connection = MagicMock() - connection.describe_images.return_value = { - "Images": [ - {"ImageId": "ami-0c7a795306730b288"}, - {"ImageId": "ami-0c7a795306730b288"}, - ] - } + m_describe_images.return_value = [ + {"ImageId": "ami-0c7a795306730b288"}, + {"ImageId": "ami-0c7a795306730b289"}, + ] with pytest.raises(ec2_ami.Ec2AmiFailure): ec2_ami.get_image_by_id(connection, "ami-0c7a795306730b288") + m_describe_images.assert_called_once_with(connection, ImageIds=["ami-0c7a795306730b288"]) -def test_get_image_missing(): +@patch(module_name + ".describe_images") +def test_get_image_missing(m_describe_images): connection = MagicMock() - connection.describe_images.return_value = {"Images": []} + m_describe_images.return_value = [] - image = ec2_ami.get_image_by_id(connection, "ami-0c7a795306730b288") - assert image is None - assert connection.describe_images.call_count == 1 - connection.describe_images.assert_has_calls( + assert ec2_ami.get_image_by_id(connection, "ami-0c7a795306730b288") is None + + assert m_describe_images.call_count == 1 + m_describe_images.assert_has_calls( [ call( - aws_retry=True, + connection, ImageIds=["ami-0c7a795306730b288"], ) ] ) +@patch( + module_name + ".create_image", +) @patch( module_name + ".get_image_by_id", ) -def test_create_image_minimal(m_get_image_by_id): +def test_create_image_minimal(m_get_image_by_id, m_create_image): module = MagicMock() connection = MagicMock() @@ -116,11 +124,11 @@ def test_create_image_minimal(m_get_image_by_id): "image_id": "ami-0c7a795306730b288", } ec2_ami.CreateImage.do(module, connection, None) - assert connection.create_image.call_count == 1 - connection.create_image.assert_has_calls( + assert m_create_image.call_count == 1 + m_create_image.assert_has_calls( [ call( - aws_retry=True, + connection, InstanceId="i-123456789", Name="my-image", ) @@ -170,13 +178,14 @@ def test_rename_item_if_exists(): assert dict_object == {"Cities": {"Abidjan": "bar"}} -def test_DeregisterImage_defer_purge_snapshots(): +@patch(module_name + ".delete_snapshot") +def test_DeregisterImage_defer_purge_snapshots(m_delete_snapshot): image = {"BlockDeviceMappings": [{"Ebs": {"SnapshotId": "My_snapshot"}}, {}]} func = ec2_ami.DeregisterImage.defer_purge_snapshots(image) connection = MagicMock() assert list(func(connection)) == ["My_snapshot"] - connection.delete_snapshot.assert_called_with(aws_retry=True, SnapshotId="My_snapshot") + m_delete_snapshot.assert_called_with(connection, "My_snapshot") @patch(module_name + ".get_image_by_id") @@ -230,16 +239,17 @@ def test_UpdateImage_set_launch_permission_check_mode_with_change(): assert connection.modify_image_attribute.call_count == 0 -def test_UpdateImage_set_launch_permission_with_change(): +@patch(module_name + ".modify_image_attribute") +def test_UpdateImage_set_launch_permission_with_change(m_modify_image_attribute): connection = MagicMock() image = {"ImageId": "ami-0c7a795306730b288", "LaunchPermissions": {}} launch_permissions = {"user_ids": ["123456789012"], "group_names": ["foo", "bar"]} changed = ec2_ami.UpdateImage.set_launch_permission(connection, image, launch_permissions, check_mode=False) assert changed is True - assert connection.modify_image_attribute.call_count == 1 - connection.modify_image_attribute.assert_called_with( - aws_retry=True, - ImageId="ami-0c7a795306730b288", + assert m_modify_image_attribute.call_count == 1 + m_modify_image_attribute.assert_called_with( + connection, + image_id="ami-0c7a795306730b288", Attribute="launchPermission", LaunchPermission={ "Add": [{"Group": "bar"}, {"Group": "foo"}, {"UserId": "123456789012"}], @@ -248,7 +258,8 @@ def test_UpdateImage_set_launch_permission_with_change(): ) -def test_UpdateImage_set_description(): +@patch(module_name + ".modify_image_attribute") +def test_UpdateImage_set_description(m_modify_image_attribute): connection = MagicMock() module = MagicMock() module.check_mode = False @@ -258,10 +269,10 @@ def test_UpdateImage_set_description(): changed = ec2_ami.UpdateImage.set_description(connection, module, image, "New description") assert changed is True - assert connection.modify_image_attribute.call_count == 1 - connection.modify_image_attribute.assert_called_with( - aws_retry=True, - ImageId="ami-0c7a795306730b288", + assert m_modify_image_attribute.call_count == 1 + m_modify_image_attribute.assert_called_with( + connection, + image_id="ami-0c7a795306730b288", Attribute="Description", Description={"Value": "New description"}, ) @@ -331,30 +342,35 @@ def test_CreateImage_do_check_mode_no_change(): ) -def test_CreateImage_do_check_mode_with_change(): +@patch(module_name + ".describe_images") +def test_CreateImage_do_check_mode_with_change(m_describe_images): module = MagicMock() module.params = {"name": "my-image"} connection = MagicMock() - connection.describe_images.return_value = {"Images": []} + m_describe_images.return_value = [] ec2_ami.CreateImage.do_check_mode(module, connection, None) + m_describe_images.assert_called_once_with(connection, Filters=[{"Name": "name", "Values": [module.params["name"]]}]) module.exit_json.assert_called_with(changed=True, msg="Would have created a AMI if not in check mode.") -@patch(module_name + ".get_waiter") -def test_CreateImage_wait(m_get_waiter): +@patch(module_name + ".wait_for_resource_state") +def test_CreateImage_wait(m_wait_for_resource_state): connection = MagicMock() m_waiter = MagicMock() - m_get_waiter.return_value = m_waiter - - assert ec2_ami.CreateImage.wait(connection, wait_timeout=0, image_id=None) is None + m_wait_for_resource_state.return_value = m_waiter + module = MagicMock() + module.params = {"wait": True, "wait_timeout": 0} + assert ec2_ami.CreateImage.wait(connection, module, image_id=ANY) is None + m_wait_for_resource_state.assert_not_called() - ec2_ami.CreateImage.wait(connection, wait_timeout=600, image_id="ami-0c7a795306730b288") - assert m_waiter.wait.call_count == 1 - m_waiter.wait.assert_called_with( - ImageIds=["ami-0c7a795306730b288"], - WaiterConfig={"Delay": 15, "MaxAttempts": 40}, + module.params = {"wait": True, "wait_timeout": 600} + image_id = "ami-0c7a795306730b288" + ec2_ami.CreateImage.wait(connection, module, image_id=image_id) + assert m_wait_for_resource_state.call_count == 1 + m_wait_for_resource_state.assert_called_with( + connection, module, "image_available", delay=15, max_attempts=40, ImageIds=[image_id] ) @@ -385,16 +401,17 @@ def test_CreateImage_set_tags(m_get_image_by_id, m_add_ec2_tags): m_add_ec2_tags.assert_called_with(connection, module, "snap-066877671789bd71b", tags) -def test_CreateInage_set_launch_permissions(): +@patch(module_name + ".modify_image_attribute") +def test_CreateInage_set_launch_permissions(m_modify_image_attribute): connection = MagicMock() launch_permissions = {"user_ids": ["123456789012"], "group_names": ["foo", "bar"]} image_id = "ami-0c7a795306730b288" ec2_ami.CreateImage.set_launch_permissions(connection, launch_permissions, image_id) - assert connection.modify_image_attribute.call_count == 1 - connection.modify_image_attribute.assert_called_with( + assert m_modify_image_attribute.call_count == 1 + m_modify_image_attribute.assert_called_with( + connection, + image_id="ami-0c7a795306730b288", Attribute="LaunchPermission", - ImageId="ami-0c7a795306730b288", LaunchPermission={"Add": [{"Group": "foo"}, {"Group": "bar"}, {"UserId": "123456789012"}]}, - aws_retry=True, ) diff --git a/tests/unit/plugins/modules/test_ec2_ami_info.py b/tests/unit/plugins/modules/test_ec2_ami_info.py index a5abc77af60..087c55077cd 100644 --- a/tests/unit/plugins/modules/test_ec2_ami_info.py +++ b/tests/unit/plugins/modules/test_ec2_ami_info.py @@ -3,12 +3,10 @@ # This file is part of Ansible # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) -from unittest.mock import ANY from unittest.mock import MagicMock from unittest.mock import call from unittest.mock import patch -import botocore.exceptions import pytest from ansible.module_utils.common.dict_transformations import camel_dict_to_snake_dict @@ -50,8 +48,9 @@ def test_build_request_args(executable_users, filters, image_ids, owners, expect assert ec2_ami_info.build_request_args(executable_users, filters, image_ids, owners) == expected -def test_get_images(ec2_client): - ec2_client.describe_images.return_value = { +@patch(module_name + ".describe_images") +def test_get_images(m_describe_images): + m_describe_images.return_value = { "Images": [ { "Architecture": "x86_64", @@ -78,93 +77,72 @@ def test_get_images(ec2_client): } request_args = {"ImageIds": ["ami-1234567890"]} - - get_images_result = ec2_ami_info.get_images(ec2_client, request_args) - - ec2_client.describe_images.call_count == 2 - ec2_client.describe_images.assert_called_with(aws_retry=True, **request_args) - assert get_images_result == ec2_client.describe_images.return_value - - -def test_get_image_attribute(): ec2_client = MagicMock() + get_images_result = ec2_ami_info.get_images(ec2_client, request_args) - ec2_client.describe_image_attribute.return_value = { - "ImageId": "ami-1234567890", - "LaunchPermissions": [{"UserId": "1234567890"}, {"UserId": "0987654321"}], - } - - image_id = "ami-1234567890" - - get_image_attribute_result = ec2_ami_info.get_image_attribute(ec2_client, image_id) - - ec2_client.describe_image_attribute.call_count == 1 - ec2_client.describe_image_attribute.assert_called_with( - aws_retry=True, Attribute="launchPermission", ImageId=image_id - ) - assert len(get_image_attribute_result["LaunchPermissions"]) == 2 + m_describe_images.call_count == 2 + m_describe_images.assert_called_with(ec2_client, **request_args) + assert get_images_result == m_describe_images.return_value -@patch(module_name + ".get_image_attribute") +@patch(module_name + ".describe_image_attribute") @patch(module_name + ".get_images") -def test_list_ec2_images(m_get_images, m_get_image_attribute): +def test_list_ec2_images(m_get_images, m_describe_image_attribute): module = MagicMock() - m_get_images.return_value = { - "Images": [ - { - "Architecture": "x86_64", - "BlockDeviceMappings": [ - { - "DeviceName": "/dev/sda1", - "Ebs": { - "DeleteOnTermination": "True", - "Encrypted": "False", - "SnapshotId": "snap-0f00cba784af62428", - "VolumeSize": 10, - "VolumeType": "gp2", - }, - } - ], - "ImageId": "ami-1234567890", - "ImageLocation": "1234567890/test-ami-uefi-boot", - "ImageType": "machine", - "Name": "test-ami-uefi-boot", - "OwnerId": "1234567890", - "OwnerAlias": "test_ami_owner", - "PlatformDetails": "Linux/UNIX", - }, - { - "Architecture": "x86_64", - "BlockDeviceMappings": [ - { - "DeviceName": "/dev/sda1", - "Ebs": { - "DeleteOnTermination": "True", - "Encrypted": "False", - "SnapshotId": "snap-0f00cba784af62428", - "VolumeSize": 10, - "VolumeType": "gp2", - }, - } - ], - "ImageId": "ami-1523498760", - "ImageLocation": "1523498760/test-ami-uefi-boot", - "ImageType": "machine", - "Name": "test-ami-uefi-boot", - "OwnerId": "1234567890", - "OwnerAlias": "test_ami_owner", - "PlatformDetails": "Linux/UNIX", - }, - ], - } - - m_get_image_attribute.return_value = { + m_get_images.return_value = [ + { + "Architecture": "x86_64", + "BlockDeviceMappings": [ + { + "DeviceName": "/dev/sda1", + "Ebs": { + "DeleteOnTermination": "True", + "Encrypted": "False", + "SnapshotId": "snap-0f00cba784af62428", + "VolumeSize": 10, + "VolumeType": "gp2", + }, + } + ], + "ImageId": "ami-1234567890", + "ImageLocation": "1234567890/test-ami-uefi-boot", + "ImageType": "machine", + "Name": "test-ami-uefi-boot", + "OwnerId": "1234567890", + "OwnerAlias": "test_ami_owner", + "PlatformDetails": "Linux/UNIX", + }, + { + "Architecture": "x86_64", + "BlockDeviceMappings": [ + { + "DeviceName": "/dev/sda1", + "Ebs": { + "DeleteOnTermination": "True", + "Encrypted": "False", + "SnapshotId": "snap-0f00cba784af62428", + "VolumeSize": 10, + "VolumeType": "gp2", + }, + } + ], + "ImageId": "ami-1523498760", + "ImageLocation": "1523498760/test-ami-uefi-boot", + "ImageType": "machine", + "Name": "test-ami-uefi-boot", + "OwnerId": "1234567890", + "OwnerAlias": "test_ami_owner", + "PlatformDetails": "Linux/UNIX", + }, + ] + + m_describe_image_attribute.return_value = { "ImageId": "ami-1234567890", "LaunchPermissions": [{"UserId": "1234567890"}, {"UserId": "0987654321"}], } - images = m_get_images.return_value["Images"] + images = m_get_images.return_value images = [camel_dict_to_snake_dict(image) for image in images] request_args = { @@ -182,10 +160,10 @@ def test_list_ec2_images(m_get_images, m_get_image_attribute): assert m_get_images.call_count == 1 m_get_images.assert_called_with(ec2_client, request_args) - assert m_get_image_attribute.call_count == 2 - m_get_image_attribute.assert_has_calls( - [call(ec2_client, images[0]["image_id"])], - [call(ec2_client, images[1]["image_id"])], + assert m_describe_image_attribute.call_count == 2 + m_describe_image_attribute.assert_has_calls( + [call(ec2_client, attribute="launchPermission", image_id=images[0]["image_id"])], + [call(ec2_client, attribute="launchPermission", image_id=images[1]["image_id"])], ) assert len(list_ec2_images_result) == 2 @@ -200,25 +178,18 @@ def test_main_success(m_AnsibleAWSModule): ec2_ami_info.main() - m_module.client.assert_called_with("ec2", retry_decorator=ANY) + m_module.client.assert_called_with("ec2") m_module.exit_json.assert_called_with(images=[]) -def a_boto_exception(): - return botocore.exceptions.UnknownServiceError(service_name="Whoops", known_service_names="Oula") +def a_ami_info_exception(): + return ec2_ami_info.AnsibleEC2Error(message="Whoops") -def test_api_failure_get_images(ec2_client): +@patch(module_name + ".describe_images") +def test_api_failure_get_images(m_describe_images): request_args = {} - ec2_client.describe_images.side_effect = a_boto_exception() + m_describe_images.side_effect = a_ami_info_exception() with pytest.raises(ec2_ami_info.AmiInfoFailure): ec2_ami_info.get_images(ec2_client, request_args) - - -def test_api_failure_get_image_attribute(ec2_client): - image_id = "ami-1234567890" - ec2_client.describe_image_attribute.side_effect = a_boto_exception() - - with pytest.raises(ec2_ami_info.AmiInfoFailure): - ec2_ami_info.get_image_attribute(ec2_client, image_id)