From 00a4249b74c1816bc468b480ae1ff6ff27351ff8 Mon Sep 17 00:00:00 2001 From: William Richard Date: Tue, 5 Dec 2017 15:47:04 -0500 Subject: [PATCH 1/3] Make test_amis not executable, so nose runs it In trying to debug changes to the ami mock introduced in 1.1.25, I noticed that the ami tests were not running. Turns out that nose does not run test files that are executable. http://nose.readthedocs.io/en/latest/finding_tests.html The ami test file was the only test file I could find that had the executable bit set. --- tests/test_ec2/test_amis.py | 0 1 file changed, 0 insertions(+), 0 deletions(-) mode change 100755 => 100644 tests/test_ec2/test_amis.py diff --git a/tests/test_ec2/test_amis.py b/tests/test_ec2/test_amis.py old mode 100755 new mode 100644 From e0d4728c5d2a149c8b6cd74d4d8a3cf0c294bc73 Mon Sep 17 00:00:00 2001 From: William Richard Date: Tue, 5 Dec 2017 16:53:30 -0500 Subject: [PATCH 2/3] Fix ami tests - missing and malformed image ids - test_ami_filters - test_ami_copy tests - test_ami_create_and_delete test - test_ami_filter_wildcard test - the rest of the tests by using the non-deprecated mock_ec2 --- moto/ec2/models.py | 71 +++++++++++------------ moto/ec2/responses/amis.py | 4 +- tests/test_ec2/test_amis.py | 110 +++++++++++++++++++++--------------- 3 files changed, 102 insertions(+), 83 deletions(-) diff --git a/moto/ec2/models.py b/moto/ec2/models.py index f22e39b8b..edf6087b9 100755 --- a/moto/ec2/models.py +++ b/moto/ec2/models.py @@ -48,7 +48,6 @@ from .exceptions import ( InvalidRouteError, InvalidInstanceIdError, InvalidAMIIdError, - MalformedAMIIdError, InvalidAMIAttributeItemValueError, InvalidSnapshotIdError, InvalidVolumeIdError, @@ -68,8 +67,8 @@ from .exceptions import ( InvalidCustomerGatewayIdError, RulesPerSecurityGroupLimitExceededError, MotoNotImplementedError, - FilterNotImplementedError -) + FilterNotImplementedError, + MalformedAMIIdError) from .utils import ( EC2_RESOURCE_TO_PREFIX, EC2_PREFIX_TO_RESOURCE, @@ -1032,11 +1031,11 @@ class TagBackend(object): class Ami(TaggedEC2Resource): def __init__(self, ec2_backend, ami_id, instance=None, source_ami=None, - name=None, description=None, owner_id=None, + name=None, description=None, owner_id=111122223333, public=False, virtualization_type=None, architecture=None, state='available', creation_date=None, platform=None, image_type='machine', image_location=None, hypervisor=None, - root_device_type=None, root_device_name=None, sriov='simple', + root_device_type='standard', root_device_name='/dev/sda1', sriov='simple', region_name='us-east-1a' ): self.ec2_backend = ec2_backend @@ -1137,14 +1136,13 @@ class AmiBackend(object): ami_id = ami['ami_id'] self.amis[ami_id] = Ami(self, **ami) - def create_image(self, instance_id, name=None, description=None, - context=None): + def create_image(self, instance_id, name=None, description=None, context=None): # TODO: check that instance exists and pull info from it. ami_id = random_ami_id() instance = self.get_instance(instance_id) ami = Ami(self, ami_id, instance=instance, source_ami=None, name=name, description=description, - owner_id=context.get_current_user() if context else None) + owner_id=context.get_current_user() if context else '111122223333') self.amis[ami_id] = ami return ami @@ -1161,36 +1159,39 @@ class AmiBackend(object): context=None): images = self.amis.values() - # Limit images by launch permissions - if exec_users: - tmp_images = [] - for ami in images: - for user_id in exec_users: - if user_id in ami.launch_permission_users: - tmp_images.append(ami) - images = tmp_images + if len(ami_ids): + # boto3 seems to default to just searching based on ami ids if that parameter is passed + # and if no images are found, it raises an errors + malformed_ami_ids = [ami_id for ami_id in ami_ids if not ami_id.startswith('ami-')] + if malformed_ami_ids: + raise MalformedAMIIdError(malformed_ami_ids) - # Limit by owner ids - if owners: - # support filtering by Owners=['self'] - owners = list(map( - lambda o: context.get_current_user() - if context and o == 'self' else o, - owners)) - images = [ami for ami in images if ami.owner_id in owners] - - if ami_ids: images = [ami for ami in images if ami.id in ami_ids] - if len(ami_ids) > len(images): - unknown_ids = set(ami_ids) - set(images) - for id in unknown_ids: - if not self.AMI_REGEX.match(id): - raise MalformedAMIIdError(id) - raise InvalidAMIIdError(unknown_ids) + if len(images) == 0: + raise InvalidAMIIdError(ami_ids) + else: + # Limit images by launch permissions + if exec_users: + tmp_images = [] + for ami in images: + for user_id in exec_users: + if user_id in ami.launch_permission_users: + tmp_images.append(ami) + images = tmp_images + + # Limit by owner ids + if owners: + # support filtering by Owners=['self'] + owners = list(map( + lambda o: context.get_current_user() + if context and o == 'self' else o, + owners)) + images = [ami for ami in images if ami.owner_id in owners] + + # Generic filters + if filters: + return generic_filter(filters, images) - # Generic filters - if filters: - return generic_filter(filters, images) return images def deregister_image(self, ami_id): diff --git a/moto/ec2/responses/amis.py b/moto/ec2/responses/amis.py index 12ab3e6be..17e1e228d 100755 --- a/moto/ec2/responses/amis.py +++ b/moto/ec2/responses/amis.py @@ -113,12 +113,12 @@ DESCRIBE_IMAGES_RESPONSE = """ Date: Thu, 11 Jan 2018 14:51:42 -0500 Subject: [PATCH 3/3] Fix tests that were introduced in PR #1398 --- moto/core/responses.py | 2 +- moto/ec2/models.py | 1 + tests/test_ec2/test_amis.py | 33 ++++++++++++++++++++------------- 3 files changed, 22 insertions(+), 14 deletions(-) diff --git a/moto/core/responses.py b/moto/core/responses.py index 5afe5e168..d254d1f85 100644 --- a/moto/core/responses.py +++ b/moto/core/responses.py @@ -192,7 +192,7 @@ class BaseResponse(_TemplateEnvironmentMixin): return self.querystring.get('AWSAccessKeyId') else: # Should we raise an unauthorized exception instead? - return None + return '111122223333' def _dispatch(self, request, full_url, headers): self.setup_class(request, full_url, headers) diff --git a/moto/ec2/models.py b/moto/ec2/models.py index edf6087b9..f877d3772 100755 --- a/moto/ec2/models.py +++ b/moto/ec2/models.py @@ -1140,6 +1140,7 @@ class AmiBackend(object): # TODO: check that instance exists and pull info from it. ami_id = random_ami_id() instance = self.get_instance(instance_id) + ami = Ami(self, ami_id, instance=instance, source_ami=None, name=name, description=description, owner_id=context.get_current_user() if context else '111122223333') diff --git a/tests/test_ec2/test_amis.py b/tests/test_ec2/test_amis.py index d3a36e423..3f21ca20b 100644 --- a/tests/test_ec2/test_amis.py +++ b/tests/test_ec2/test_amis.py @@ -7,6 +7,7 @@ from boto.exception import EC2ResponseError from botocore.exceptions import ClientError # Ensure 'assert_raises' context manager support for Python 2.6 from nose.tools import assert_raises +import sure # noqa from moto import mock_ec2_deprecated, mock_ec2 from tests.helpers import requires_boto_gte @@ -695,16 +696,20 @@ def test_ami_describe_non_existent(): @mock_ec2 def test_ami_filter_wildcard(): - ec2 = boto3.resource('ec2', region_name='us-west-1') - instance = ec2.create_instances(ImageId='ami-1234abcd', MinCount=1, MaxCount=1)[0] - image = instance.create_image(Name='test-image') + ec2_resource = boto3.resource('ec2', region_name='us-west-1') + ec2_client = boto3.client('ec2', region_name='us-west-1') + + instance = ec2_resource.create_instances(ImageId='ami-1234abcd', MinCount=1, MaxCount=1)[0] + instance.create_image(Name='test-image') # create an image with the same owner but will not match the filter instance.create_image(Name='not-matching-image') - filter_result = list( - ec2.images.filter(Owners=['111122223333'], Filters=[{'Name': 'name', 'Values': ['test*']}])) - filter_result.should.equal([image]) + my_images = ec2_client.describe_images( + Owners=['111122223333'], + Filters=[{'Name': 'name', 'Values': ['test*']}] + )['Images'] + my_images.should.have.length_of(1) @mock_ec2 @@ -724,16 +729,18 @@ def test_ami_filter_by_owner_id(): # Check we actually have a subset of images assert len(ubuntu_ids) < len(all_ids) + @mock_ec2 def test_ami_filter_by_self(): - client = boto3.client('ec2', region_name='us-east-1') + ec2_resource = boto3.resource('ec2', region_name='us-west-1') + ec2_client = boto3.client('ec2', region_name='us-west-1') - my_images = client.describe_images(Owners=['self']) - assert len(my_images) == 0 + my_images = ec2_client.describe_images(Owners=['self'])['Images'] + my_images.should.have.length_of(0) # Create a new image - instance = ec2.create_instances(ImageId='ami-1234abcd', MinCount=1, MaxCount=1)[0] - image = instance.create_image(Name='test-image') + instance = ec2_resource.create_instances(ImageId='ami-1234abcd', MinCount=1, MaxCount=1)[0] + instance.create_image(Name='test-image') - my_images = client.describe_images(Owners=['self']) - assert len(my_images) == 1 + my_images = ec2_client.describe_images(Owners=['self'])['Images'] + my_images.should.have.length_of(1)