From 1a176c67045be66d79fb35fb182ed55fd9879432 Mon Sep 17 00:00:00 2001 From: Mandar Kulkarni Date: Thu, 3 Nov 2022 14:17:20 -0700 Subject: [PATCH 01/10] Refactor: add method to build API request args --- plugins/modules/ec2_ami_info.py | 43 +++++++++++++++++++++------------ 1 file changed, 28 insertions(+), 15 deletions(-) diff --git a/plugins/modules/ec2_ami_info.py b/plugins/modules/ec2_ami_info.py index 3d67e89de6c..4c20ec3515a 100644 --- a/plugins/modules/ec2_ami_info.py +++ b/plugins/modules/ec2_ami_info.py @@ -215,13 +215,14 @@ from ansible_collections.amazon.aws.plugins.module_utils.ec2 import boto3_tag_list_to_ansible_dict -def list_ec2_images(ec2_client, module): +def build_request_args(executable_users, filters, image_ids, owners): - image_ids = module.params.get("image_ids") - owners = module.params.get("owners") - executable_users = module.params.get("executable_users") - filters = module.params.get("filters") - owner_param = [] + request_args = { + 'ExecutableUsers': [str(user) for user in executable_users], + 'Filters': ansible_dict_to_boto3_filter_list(filters), + 'ImageIds': [str(image_id) for image_id in image_ids], + 'Owners': [str(owner) for owner in owners], + } # describe_images is *very* slow if you pass the `Owners` # param (unless it's self), for some reason. @@ -235,17 +236,20 @@ def list_ec2_images(ec2_client, module): filters['owner-id'].append(owner) elif owner == 'self': # self not a valid owner-alias filter (https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_DescribeImages.html) - owner_param.append(owner) + request_args['Owners'].append(owner) else: if 'owner-alias' not in filters: filters['owner-alias'] = list() filters['owner-alias'].append(owner) - filters = ansible_dict_to_boto3_filter_list(filters) + request_args = {k: v for k, v in request_args.items() if v} + return request_args + + +def list_ec2_images(ec2_client, module, request_args): try: - images = ec2_client.describe_images(aws_retry=True, ImageIds=image_ids, Filters=filters, Owners=owner_param, - ExecutableUsers=executable_users) + images = ec2_client.describe_images(aws_retry=True, **request_args) images = [camel_dict_to_snake_dict(image) for image in images["Images"]] except (ClientError, BotoCoreError) as err: module.fail_json_aws(err, msg="error describing images") @@ -263,25 +267,34 @@ def list_ec2_images(ec2_client, module): module.fail_json_aws(err, 'Failed to describe AMI') images.sort(key=lambda e: e.get('creation_date', '')) # it may be possible that creation_date does not always exist - module.exit_json(images=images) + + return images def main(): argument_spec = dict( - image_ids=dict(default=[], type='list', elements='str', aliases=['image_id']), + describe_image_attributes=dict(default=False, type='bool'), + executable_users=dict(default=[], type='list', elements='str', aliases=['executable_user']), filters=dict(default={}, type='dict'), + image_ids=dict(default=[], type='list', elements='str', aliases=['image_id']), owners=dict(default=[], type='list', elements='str', aliases=['owner']), - executable_users=dict(default=[], type='list', elements='str', aliases=['executable_user']), - describe_image_attributes=dict(default=False, type='bool') ) module = AnsibleAWSModule(argument_spec=argument_spec, supports_check_mode=True) ec2_client = module.client('ec2', retry_decorator=AWSRetry.jittered_backoff()) - list_ec2_images(ec2_client, module) + request_args = build_request_args( + executable_users = module.params["executable_users"], + filters = module.params["filters"], + image_ids = module.params["image_ids"], + owners = module.params["owners"], + ) + images = list_ec2_images(ec2_client, module, request_args) + + module.exit_json(images=images) if __name__ == '__main__': main() From e3024e415ad48019a3cb455e89faab4df3ecfa12 Mon Sep 17 00:00:00 2001 From: Mandar Kulkarni Date: Thu, 3 Nov 2022 14:29:38 -0700 Subject: [PATCH 02/10] Move describe_images to separate function --- plugins/modules/ec2_ami_info.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/plugins/modules/ec2_ami_info.py b/plugins/modules/ec2_ami_info.py index 4c20ec3515a..8542ac046b9 100644 --- a/plugins/modules/ec2_ami_info.py +++ b/plugins/modules/ec2_ami_info.py @@ -247,12 +247,20 @@ def build_request_args(executable_users, filters, image_ids, owners): return request_args -def list_ec2_images(ec2_client, module, request_args): +def get_images(ec2_client, module, request_args): try: images = ec2_client.describe_images(aws_retry=True, **request_args) - images = [camel_dict_to_snake_dict(image) for image in images["Images"]] except (ClientError, BotoCoreError) as err: module.fail_json_aws(err, msg="error describing images") + + return images + + +def list_ec2_images(ec2_client, module, request_args): + + images = get_images(ec2_client, module, request_args) + images = [camel_dict_to_snake_dict(image) for image in images["Images"]] + for image in images: try: image['tags'] = boto3_tag_list_to_ansible_dict(image.get('tags', [])) From 9a54de4b3e364ca6244342a41a41c012103a0682 Mon Sep 17 00:00:00 2001 From: Mandar Kulkarni Date: Thu, 3 Nov 2022 14:39:23 -0700 Subject: [PATCH 03/10] Move describe_image_attribute to separate function --- plugins/modules/ec2_ami_info.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/plugins/modules/ec2_ami_info.py b/plugins/modules/ec2_ami_info.py index 8542ac046b9..c206fb1c8dc 100644 --- a/plugins/modules/ec2_ami_info.py +++ b/plugins/modules/ec2_ami_info.py @@ -252,10 +252,14 @@ def get_images(ec2_client, module, request_args): images = ec2_client.describe_images(aws_retry=True, **request_args) except (ClientError, BotoCoreError) as err: module.fail_json_aws(err, msg="error describing images") - return images +def get_image_attribute(ec2_client, image): + launch_permissions = ec2_client.describe_image_attribute(aws_retry=True, Attribute='launchPermission', + ImageId=image['image_id'])['LaunchPermissions'] + return launch_permissions + def list_ec2_images(ec2_client, module, request_args): images = get_images(ec2_client, module, request_args) @@ -265,8 +269,7 @@ def list_ec2_images(ec2_client, module, request_args): try: image['tags'] = boto3_tag_list_to_ansible_dict(image.get('tags', [])) if module.params.get("describe_image_attributes"): - launch_permissions = ec2_client.describe_image_attribute(aws_retry=True, Attribute='launchPermission', - ImageId=image['image_id'])['LaunchPermissions'] + launch_permissions = get_image_attribute(ec2_client, image) image['launch_permissions'] = [camel_dict_to_snake_dict(perm) for perm in launch_permissions] except is_boto3_error_code('AuthFailure'): # describing launch permissions of images owned by others is not permitted, but shouldn't cause failures From 274f12130b529ab04278d6bd9c7c0c33d26b838c Mon Sep 17 00:00:00 2001 From: Mandar Kulkarni Date: Thu, 3 Nov 2022 14:57:12 -0700 Subject: [PATCH 04/10] Sanity fixes --- plugins/modules/ec2_ami_info.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/plugins/modules/ec2_ami_info.py b/plugins/modules/ec2_ami_info.py index c206fb1c8dc..e4f5b5aeceb 100644 --- a/plugins/modules/ec2_ami_info.py +++ b/plugins/modules/ec2_ami_info.py @@ -256,10 +256,11 @@ def get_images(ec2_client, module, request_args): def get_image_attribute(ec2_client, image): - launch_permissions = ec2_client.describe_image_attribute(aws_retry=True, Attribute='launchPermission', - ImageId=image['image_id'])['LaunchPermissions'] + launch_permissions = ec2_client.describe_image_attribute( + aws_retry=True, Attribute='launchPermission', ImageId=image['image_id'])['LaunchPermissions'] return launch_permissions + def list_ec2_images(ec2_client, module, request_args): images = get_images(ec2_client, module, request_args) @@ -297,15 +298,16 @@ def main(): ec2_client = module.client('ec2', retry_decorator=AWSRetry.jittered_backoff()) request_args = build_request_args( - executable_users = module.params["executable_users"], - filters = module.params["filters"], - image_ids = module.params["image_ids"], - owners = module.params["owners"], + executable_users=module.params["executable_users"], + filters=module.params["filters"], + image_ids=module.params["image_ids"], + owners=module.params["owners"], ) images = list_ec2_images(ec2_client, module, request_args) module.exit_json(images=images) + if __name__ == '__main__': main() From ab87a25c1ffed5ad5514dbd9992e55595fdc75ae Mon Sep 17 00:00:00 2001 From: Mandar Kulkarni Date: Thu, 3 Nov 2022 19:29:29 -0700 Subject: [PATCH 05/10] Add test for build_request_args() --- .../unit/plugins/modules/test_ec2_ami_info.py | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 tests/unit/plugins/modules/test_ec2_ami_info.py diff --git a/tests/unit/plugins/modules/test_ec2_ami_info.py b/tests/unit/plugins/modules/test_ec2_ami_info.py new file mode 100644 index 00000000000..f1f40302f98 --- /dev/null +++ b/tests/unit/plugins/modules/test_ec2_ami_info.py @@ -0,0 +1,21 @@ +# (c) 2022 Red Hat Inc. + +# 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 MagicMock, patch, ANY, call +import pytest + +from ansible_collections.amazon.aws.plugins.modules import ec2_ami_info + +module_name = "ansible_collections.amazon.aws.plugins.modules.ec2_ami_info" + + +@pytest.mark.parametrize("executable_users,filters,image_ids,owners,expected", [ + ([], {}, [], [], {}), + ([], {}, ['ami-1234567890'], [], {'ImageIds': ['ami-1234567890']}), + ([], {}, [], ['1234567890'], {'Filters': [{'Name': 'owner-id', 'Values': ['1234567890']}]}), + ([], {'owner-alias': 'test_ami_owner'}, [], ['1234567890'], {'Filters': [{'Name': 'owner-alias', 'Values': ['test_ami_owner']}, {'Name': 'owner-id', 'Values': ['1234567890']}]})]) +def test_build_request_args(executable_users, filters, image_ids, owners, expected): + assert ec2_ami_info.build_request_args( + executable_users, filters, image_ids, owners) == expected From cbcab4c639b0d04136f75baf53e468e872273915 Mon Sep 17 00:00:00 2001 From: Mandar Kulkarni Date: Mon, 7 Nov 2022 15:05:39 -0800 Subject: [PATCH 06/10] Fix handling in build_request_args() --- plugins/modules/ec2_ami_info.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/plugins/modules/ec2_ami_info.py b/plugins/modules/ec2_ami_info.py index e4f5b5aeceb..6787e34f359 100644 --- a/plugins/modules/ec2_ami_info.py +++ b/plugins/modules/ec2_ami_info.py @@ -217,13 +217,6 @@ def build_request_args(executable_users, filters, image_ids, owners): - request_args = { - 'ExecutableUsers': [str(user) for user in executable_users], - 'Filters': ansible_dict_to_boto3_filter_list(filters), - 'ImageIds': [str(image_id) for image_id in image_ids], - 'Owners': [str(owner) for owner in owners], - } - # describe_images is *very* slow if you pass the `Owners` # param (unless it's self), for some reason. # Converting the owners to filters and removing from the @@ -242,6 +235,12 @@ def build_request_args(executable_users, filters, image_ids, owners): filters['owner-alias'] = list() filters['owner-alias'].append(owner) + request_args = { + 'ExecutableUsers': [str(user) for user in executable_users], + 'Filters': ansible_dict_to_boto3_filter_list(filters), + 'ImageIds': [str(image_id) for image_id in image_ids], + } + request_args = {k: v for k, v in request_args.items() if v} return request_args From bf8dae78a375baaee24dccc70915acc57d0c3017 Mon Sep 17 00:00:00 2001 From: Mandar Kulkarni Date: Mon, 7 Nov 2022 20:11:24 -0800 Subject: [PATCH 07/10] Refactor and add more unit tests --- plugins/modules/ec2_ami_info.py | 37 ++- .../unit/plugins/modules/test_ec2_ami_info.py | 234 +++++++++++++++++- 2 files changed, 257 insertions(+), 14 deletions(-) diff --git a/plugins/modules/ec2_ami_info.py b/plugins/modules/ec2_ami_info.py index 6787e34f359..8b2e711bc72 100644 --- a/plugins/modules/ec2_ami_info.py +++ b/plugins/modules/ec2_ami_info.py @@ -215,8 +215,20 @@ from ansible_collections.amazon.aws.plugins.module_utils.ec2 import boto3_tag_list_to_ansible_dict +class AmiInfoFailure(Exception): + def __init__(self, original_e, user_message): + self.original_e = original_e + self.user_message = user_message + super().__init__(self) + + def build_request_args(executable_users, filters, image_ids, owners): + request_args = { + 'ExecutableUsers': [str(user) for user in executable_users], + 'ImageIds': [str(image_id) for image_id in image_ids], + } + # describe_images is *very* slow if you pass the `Owners` # param (unless it's self), for some reason. # Converting the owners to filters and removing from the @@ -229,17 +241,13 @@ def build_request_args(executable_users, filters, image_ids, owners): filters['owner-id'].append(owner) elif owner == 'self': # self not a valid owner-alias filter (https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_DescribeImages.html) - request_args['Owners'].append(owner) + request_args['Owners'] = [str(owner)] else: if 'owner-alias' not in filters: filters['owner-alias'] = list() filters['owner-alias'].append(owner) - request_args = { - 'ExecutableUsers': [str(user) for user in executable_users], - 'Filters': ansible_dict_to_boto3_filter_list(filters), - 'ImageIds': [str(image_id) for image_id in image_ids], - } + request_args['Filters'] = ansible_dict_to_boto3_filter_list(filters) request_args = {k: v for k, v in request_args.items() if v} @@ -250,32 +258,35 @@ def get_images(ec2_client, module, request_args): try: images = ec2_client.describe_images(aws_retry=True, **request_args) except (ClientError, BotoCoreError) as err: - module.fail_json_aws(err, msg="error describing images") + raise AmiInfoFailure(err, "error describing images") return images def get_image_attribute(ec2_client, image): - launch_permissions = ec2_client.describe_image_attribute( - aws_retry=True, Attribute='launchPermission', ImageId=image['image_id'])['LaunchPermissions'] + try: + launch_permissions = ec2_client.describe_image_attribute( + aws_retry=True, Attribute='launchPermission', ImageId=image['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, module, request_args) - images = [camel_dict_to_snake_dict(image) for image in images["Images"]] + images = get_images(ec2_client, module, request_args)["Images"] + images = [camel_dict_to_snake_dict(image) for image in images] for image in images: try: 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) + launch_permissions = get_image_attribute(ec2_client, image)['LaunchPermissions'] image['launch_permissions'] = [camel_dict_to_snake_dict(perm) for perm in launch_permissions] except is_boto3_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 - module.fail_json_aws(err, 'Failed to describe AMI') + raise AmiInfoFailure(err, 'Failed to describe AMI') images.sort(key=lambda e: e.get('creation_date', '')) # it may be possible that creation_date does not always exist diff --git a/tests/unit/plugins/modules/test_ec2_ami_info.py b/tests/unit/plugins/modules/test_ec2_ami_info.py index f1f40302f98..f8e7c5d1bb9 100644 --- a/tests/unit/plugins/modules/test_ec2_ami_info.py +++ b/tests/unit/plugins/modules/test_ec2_ami_info.py @@ -3,7 +3,9 @@ # 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 ansible.module_utils.common.dict_transformations import camel_dict_to_snake_dict from unittest.mock import MagicMock, patch, ANY, call +import botocore.exceptions import pytest from ansible_collections.amazon.aws.plugins.modules import ec2_ami_info @@ -15,7 +17,237 @@ ([], {}, [], [], {}), ([], {}, ['ami-1234567890'], [], {'ImageIds': ['ami-1234567890']}), ([], {}, [], ['1234567890'], {'Filters': [{'Name': 'owner-id', 'Values': ['1234567890']}]}), - ([], {'owner-alias': 'test_ami_owner'}, [], ['1234567890'], {'Filters': [{'Name': 'owner-alias', 'Values': ['test_ami_owner']}, {'Name': 'owner-id', 'Values': ['1234567890']}]})]) + ([], {'owner-alias': 'test_ami_owner'}, [], ['1234567890'], {'Filters': [{'Name': 'owner-alias', + 'Values': ['test_ami_owner']}, {'Name': 'owner-id', 'Values': ['1234567890']}]}), + ([], {'is-public': True}, [], [], {'Filters': [{'Name': 'is-public', 'Values': ['true']}]}), + ([], {}, [], ['self'], {'Owners': ['self']})]) def test_build_request_args(executable_users, filters, image_ids, owners, expected): assert ec2_ami_info.build_request_args( executable_users, filters, image_ids, owners) == expected + + +def test_get_images(): + ec2_client = MagicMock() + module = MagicMock() + + ec2_client.describe_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", + "PlatformDetails": "Linux/UNIX", + } + ], + } + + request_args = {'ImageIds': ['ami-1234567890']} + + get_images_result = ec2_ami_info.get_images(ec2_client, module, request_args) + + ec2_client.describe_images.call_count == 2 + ec2_client.describe_images.assert_called_with(aws_retry=True, **request_args) + assert len(get_images_result['Images']) == 1 + + +def test_get_image_attribute(): + ec2_client = MagicMock() + + ec2_client.describe_image_attribute.return_value = { + "ImageId": "ami-1234567890", + "LaunchPermissions": [ + { + "UserId": "1234567890" + }, + { + "UserId": "0987654321" + } + ] + } + + image = { + "architecture": "x86_64", + "blockDeviceMappings": [ + { + "device_name": "/dev/sda1", + "ebs": { + "delete_on_termination": "True", + "encrypted": "False", + "snapshot_id": "snap-0f00cba784af62428", + "volume_size": 10, + "volume_Type": "gp2" + } + } + ], + "image_id": "ami-1234567890", + "image_location": "1234567890/test-ami-uefi-boot", + "image_type": "machine", + "name": "test-ami-uefi-boot", + "owner_id": "1234567890", + "platform_details": "Linux/UNIX" + } + + get_image_attribute_result = ec2_ami_info.get_image_attribute(ec2_client, image) + + ec2_client.describe_image_attribute.call_count == 1 + ec2_client.describe_image_attribute.assert_called_with(aws_retry=True, Attribute='launchPermission', ImageId=image['image_id']) + assert len(get_image_attribute_result['LaunchPermissions']) == 2 + + +@patch(module_name + '.get_image_attribute') +@patch(module_name + '.get_images') +def test_list_ec2_images(m_get_images, m_get_image_attribute): + ec2_client = MagicMock() + 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 = { + "ImageId": "ami-1234567890", + "LaunchPermissions": [ + { + "UserId": "1234567890" + }, + { + "UserId": "0987654321" + } + ] + } + + images = m_get_images.return_value["Images"] + images = [camel_dict_to_snake_dict(image) for image in images] + + request_args = { + 'Filters': [ + { + 'Name': 'owner-alias', + 'Values': ['test_ami_owner'] + }, + { + 'Name': 'owner-id', + 'Values': ['1234567890'] + } + ] + } + + # needed for `assert m_get_image_attribute.call_count == 2` + module.params = { + "describe_image_attributes": True + } + + list_ec2_images_result = ec2_ami_info.list_ec2_images(ec2_client, module, request_args) + + assert m_get_images.call_count == 1 + m_get_images.assert_called_with(ec2_client, module, request_args) + + assert m_get_image_attribute.call_count == 2 + assert m_get_image_attribute.has_calls( + [ + call(ec2_client, images[0]) + ], + [ + call(ec2_client, images[1]) + ] + ) + + assert len(list_ec2_images_result) == 2 + assert list_ec2_images_result[0]["image_id"] == "ami-1234567890" + assert list_ec2_images_result[1]["image_id"] == "ami-1523498760" + + +@patch(module_name + ".AnsibleAWSModule") +def test_main_success(m_AnsibleAWSModule): + m_module = MagicMock() + m_AnsibleAWSModule.return_value = m_module + + ec2_ami_info.main() + + m_module.client.assert_called_with("ec2", retry_decorator=ANY) + m_module.exit_json.assert_called_with(images=[]) + + +def a_boto_exception(): + return botocore.exceptions.UnknownServiceError( + service_name="Whoops", known_service_names="Oula" + ) + + +def test_api_failure_get_images(): + ec2_client = MagicMock() + module = MagicMock() + request_args = {} + ec2_client.describe_images.side_effect = a_boto_exception() + + with pytest.raises(ec2_ami_info.AmiInfoFailure): + ec2_ami_info.get_images(ec2_client, module, request_args) + + +def test_api_failure_get_image_attribute(): + ec2_client = MagicMock() + image = {'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) From 62442efea9c827138fa133ea34a72102febb1d18 Mon Sep 17 00:00:00 2001 From: Mandar Kulkarni Date: Mon, 7 Nov 2022 20:21:19 -0800 Subject: [PATCH 08/10] minor fix --- plugins/modules/ec2_ami_info.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/modules/ec2_ami_info.py b/plugins/modules/ec2_ami_info.py index 8b2e711bc72..d90feafeb73 100644 --- a/plugins/modules/ec2_ami_info.py +++ b/plugins/modules/ec2_ami_info.py @@ -254,7 +254,7 @@ def build_request_args(executable_users, filters, image_ids, owners): return request_args -def get_images(ec2_client, module, request_args): +def get_images(ec2_client, request_args): try: images = ec2_client.describe_images(aws_retry=True, **request_args) except (ClientError, BotoCoreError) as err: From ba77fcab7c627dd2f2332bcb02ad7a49c5f2b80b Mon Sep 17 00:00:00 2001 From: Mandar Kulkarni Date: Mon, 7 Nov 2022 20:22:41 -0800 Subject: [PATCH 09/10] Added changelogs fragment --- changelogs/fragments/unit-tests_test_ec2_ami_info_only.yaml | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelogs/fragments/unit-tests_test_ec2_ami_info_only.yaml diff --git a/changelogs/fragments/unit-tests_test_ec2_ami_info_only.yaml b/changelogs/fragments/unit-tests_test_ec2_ami_info_only.yaml new file mode 100644 index 00000000000..565d6b8c228 --- /dev/null +++ b/changelogs/fragments/unit-tests_test_ec2_ami_info_only.yaml @@ -0,0 +1,3 @@ +--- +minor_changes: +- "ec2_ami_info - Add unit-tests coverage (https://github.com/ansible-collections/amazon.aws/pull/1252)." \ No newline at end of file From 63c90e996dadeef2012496d576f908c0f30070fb Mon Sep 17 00:00:00 2001 From: Mandar Kulkarni Date: Fri, 11 Nov 2022 18:38:51 -0800 Subject: [PATCH 10/10] Modified based on feedback --- plugins/modules/ec2_ami_info.py | 4 +-- .../unit/plugins/modules/test_ec2_ami_info.py | 26 +++++++++---------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/plugins/modules/ec2_ami_info.py b/plugins/modules/ec2_ami_info.py index d90feafeb73..c173ebc0649 100644 --- a/plugins/modules/ec2_ami_info.py +++ b/plugins/modules/ec2_ami_info.py @@ -273,14 +273,14 @@ def get_image_attribute(ec2_client, image): def list_ec2_images(ec2_client, module, request_args): - images = get_images(ec2_client, module, request_args)["Images"] + images = get_images(ec2_client, request_args)["Images"] images = [camel_dict_to_snake_dict(image) for image in images] for image in images: try: 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)['LaunchPermissions'] + launch_permissions = get_image_attribute(ec2_client, image).get('LaunchPermissions', []) image['launch_permissions'] = [camel_dict_to_snake_dict(perm) for perm in launch_permissions] except is_boto3_error_code('AuthFailure'): # describing launch permissions of images owned by others is not permitted, but shouldn't cause failures diff --git a/tests/unit/plugins/modules/test_ec2_ami_info.py b/tests/unit/plugins/modules/test_ec2_ami_info.py index f8e7c5d1bb9..f7fbc40a7cd 100644 --- a/tests/unit/plugins/modules/test_ec2_ami_info.py +++ b/tests/unit/plugins/modules/test_ec2_ami_info.py @@ -13,6 +13,11 @@ module_name = "ansible_collections.amazon.aws.plugins.modules.ec2_ami_info" +@pytest.fixture +def ec2_client(): + return MagicMock() + + @pytest.mark.parametrize("executable_users,filters,image_ids,owners,expected", [ ([], {}, [], [], {}), ([], {}, ['ami-1234567890'], [], {'ImageIds': ['ami-1234567890']}), @@ -20,15 +25,14 @@ ([], {'owner-alias': 'test_ami_owner'}, [], ['1234567890'], {'Filters': [{'Name': 'owner-alias', 'Values': ['test_ami_owner']}, {'Name': 'owner-id', 'Values': ['1234567890']}]}), ([], {'is-public': True}, [], [], {'Filters': [{'Name': 'is-public', 'Values': ['true']}]}), + (['self'], {}, [], [], {'ExecutableUsers': ['self']}), ([], {}, [], ['self'], {'Owners': ['self']})]) def test_build_request_args(executable_users, filters, image_ids, owners, expected): assert ec2_ami_info.build_request_args( executable_users, filters, image_ids, owners) == expected -def test_get_images(): - ec2_client = MagicMock() - module = MagicMock() +def test_get_images(ec2_client): ec2_client.describe_images.return_value = { "Images": [ @@ -58,11 +62,11 @@ def test_get_images(): request_args = {'ImageIds': ['ami-1234567890']} - get_images_result = ec2_ami_info.get_images(ec2_client, module, request_args) + 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 len(get_images_result['Images']) == 1 + assert get_images_result == ec2_client.describe_images.return_value def test_get_image_attribute(): @@ -112,7 +116,6 @@ def test_get_image_attribute(): @patch(module_name + '.get_image_attribute') @patch(module_name + '.get_images') def test_list_ec2_images(m_get_images, m_get_image_attribute): - ec2_client = MagicMock() module = MagicMock() m_get_images.return_value = { @@ -200,7 +203,7 @@ def test_list_ec2_images(m_get_images, m_get_image_attribute): list_ec2_images_result = ec2_ami_info.list_ec2_images(ec2_client, module, request_args) assert m_get_images.call_count == 1 - m_get_images.assert_called_with(ec2_client, module, request_args) + m_get_images.assert_called_with(ec2_client, request_args) assert m_get_image_attribute.call_count == 2 assert m_get_image_attribute.has_calls( @@ -234,18 +237,15 @@ def a_boto_exception(): ) -def test_api_failure_get_images(): - ec2_client = MagicMock() - module = MagicMock() +def test_api_failure_get_images(ec2_client): request_args = {} ec2_client.describe_images.side_effect = a_boto_exception() with pytest.raises(ec2_ami_info.AmiInfoFailure): - ec2_ami_info.get_images(ec2_client, module, request_args) + ec2_ami_info.get_images(ec2_client, request_args) -def test_api_failure_get_image_attribute(): - ec2_client = MagicMock() +def test_api_failure_get_image_attribute(ec2_client): image = {'image_id': 'ami-1234567890'} ec2_client.describe_image_attribute.side_effect = a_boto_exception()