From 6b7294a018c44b522c7db47c43591940cdcf3364 Mon Sep 17 00:00:00 2001 From: gruebel Date: Fri, 25 Oct 2019 17:57:50 +0200 Subject: [PATCH 01/15] Fix sns.add_permission & remove_permission --- IMPLEMENTATION_COVERAGE.md | 26 +++- moto/sns/models.py | 121 +++++++++++---- moto/sns/responses.py | 25 +--- tests/test_sns/test_topics.py | 37 ++++- tests/test_sns/test_topics_boto3.py | 219 ++++++++++++++++++++++++++-- 5 files changed, 359 insertions(+), 69 deletions(-) diff --git a/IMPLEMENTATION_COVERAGE.md b/IMPLEMENTATION_COVERAGE.md index 072173226..1e7c84424 100644 --- a/IMPLEMENTATION_COVERAGE.md +++ b/IMPLEMENTATION_COVERAGE.md @@ -700,6 +700,7 @@ 0% implemented - [ ] associate_phone_number_with_user - [ ] associate_phone_numbers_with_voice_connector +- [ ] associate_phone_numbers_with_voice_connector_group - [ ] batch_delete_phone_number - [ ] batch_suspend_user - [ ] batch_unsuspend_user @@ -709,15 +710,19 @@ - [ ] create_bot - [ ] create_phone_number_order - [ ] create_voice_connector +- [ ] create_voice_connector_group - [ ] delete_account - [ ] delete_events_configuration - [ ] delete_phone_number - [ ] delete_voice_connector +- [ ] delete_voice_connector_group - [ ] delete_voice_connector_origination +- [ ] delete_voice_connector_streaming_configuration - [ ] delete_voice_connector_termination - [ ] delete_voice_connector_termination_credentials - [ ] disassociate_phone_number_from_user - [ ] disassociate_phone_numbers_from_voice_connector +- [ ] disassociate_phone_numbers_from_voice_connector_group - [ ] get_account - [ ] get_account_settings - [ ] get_bot @@ -725,10 +730,14 @@ - [ ] get_global_settings - [ ] get_phone_number - [ ] get_phone_number_order +- [ ] get_phone_number_settings - [ ] get_user - [ ] get_user_settings - [ ] get_voice_connector +- [ ] get_voice_connector_group +- [ ] get_voice_connector_logging_configuration - [ ] get_voice_connector_origination +- [ ] get_voice_connector_streaming_configuration - [ ] get_voice_connector_termination - [ ] get_voice_connector_termination_health - [ ] invite_users @@ -737,11 +746,14 @@ - [ ] list_phone_number_orders - [ ] list_phone_numbers - [ ] list_users +- [ ] list_voice_connector_groups - [ ] list_voice_connector_termination_credentials - [ ] list_voice_connectors - [ ] logout_user - [ ] put_events_configuration +- [ ] put_voice_connector_logging_configuration - [ ] put_voice_connector_origination +- [ ] put_voice_connector_streaming_configuration - [ ] put_voice_connector_termination - [ ] put_voice_connector_termination_credentials - [ ] regenerate_security_token @@ -753,9 +765,11 @@ - [ ] update_bot - [ ] update_global_settings - [ ] update_phone_number +- [ ] update_phone_number_settings - [ ] update_user - [ ] update_user_settings - [ ] update_voice_connector +- [ ] update_voice_connector_group ## cloud9 0% implemented @@ -1525,6 +1539,10 @@ - [ ] get_current_metric_data - [ ] get_federation_token - [ ] get_metric_data +- [ ] list_contact_flows +- [ ] list_hours_of_operations +- [ ] list_phone_numbers +- [ ] list_queues - [ ] list_routing_profiles - [ ] list_security_profiles - [ ] list_user_hierarchy_groups @@ -3244,7 +3262,7 @@ - [ ] describe_events ## iam -61% implemented +60% implemented - [ ] add_client_id_to_open_id_connect_provider - [X] add_role_to_instance_profile - [X] add_user_to_group @@ -6029,8 +6047,8 @@ - [ ] update_job ## sns -57% implemented -- [ ] add_permission +63% implemented +- [X] add_permission - [ ] check_if_phone_number_is_opted_out - [ ] confirm_subscription - [X] create_platform_application @@ -6053,7 +6071,7 @@ - [X] list_topics - [ ] opt_in_phone_number - [X] publish -- [ ] remove_permission +- [X] remove_permission - [X] set_endpoint_attributes - [ ] set_platform_application_attributes - [ ] set_sms_attributes diff --git a/moto/sns/models.py b/moto/sns/models.py index 90bb92754..4fcacb495 100644 --- a/moto/sns/models.py +++ b/moto/sns/models.py @@ -34,7 +34,6 @@ class Topic(BaseModel): self.sns_backend = sns_backend self.account_id = DEFAULT_ACCOUNT_ID self.display_name = "" - self.policy = json.dumps(DEFAULT_TOPIC_POLICY) self.delivery_policy = "" self.effective_delivery_policy = json.dumps(DEFAULT_EFFECTIVE_DELIVERY_POLICY) self.arn = make_arn_for_topic( @@ -44,6 +43,7 @@ class Topic(BaseModel): self.subscriptions_confimed = 0 self.subscriptions_deleted = 0 + self._policy_json = self._create_default_topic_policy(sns_backend.region_name, self.account_id, name) self._tags = {} def publish(self, message, subject=None, message_attributes=None): @@ -64,6 +64,14 @@ class Topic(BaseModel): def physical_resource_id(self): return self.arn + @property + def policy(self): + return json.dumps(self._policy_json) + + @policy.setter + def policy(self, policy): + self._policy_json = json.loads(policy) + @classmethod def create_from_cloudformation_json(cls, resource_name, cloudformation_json, region_name): sns_backend = sns_backends[region_name] @@ -77,6 +85,37 @@ class Topic(BaseModel): 'Endpoint'], subscription['Protocol']) return topic + def _create_default_topic_policy(self, region_name, account_id, name): + return { + "Version": "2008-10-17", + "Id": "__default_policy_ID", + "Statement": [{ + "Effect": "Allow", + "Sid": "__default_statement_ID", + "Principal": { + "AWS": "*" + }, + "Action": [ + "SNS:GetTopicAttributes", + "SNS:SetTopicAttributes", + "SNS:AddPermission", + "SNS:RemovePermission", + "SNS:DeleteTopic", + "SNS:Subscribe", + "SNS:ListSubscriptionsByTopic", + "SNS:Publish", + "SNS:Receive", + ], + "Resource": make_arn_for_topic( + self.account_id, name, region_name), + "Condition": { + "StringEquals": { + "AWS:SourceOwner": str(account_id) + } + } + }] + } + class Subscription(BaseModel): @@ -269,7 +308,6 @@ class SNSBackend(BaseBackend): self.region_name = region_name self.sms_attributes = {} self.opt_out_numbers = ['+447420500600', '+447420505401', '+447632960543', '+447632960028', '+447700900149', '+447700900550', '+447700900545', '+447700900907'] - self.permissions = {} def reset(self): region_name = self.region_name @@ -511,6 +549,43 @@ class SNSBackend(BaseBackend): raise SNSInvalidParameter("Invalid parameter: FilterPolicy: Match value must be String, number, true, false, or null") + def add_permission(self, topic_arn, label, aws_account_ids, action_names): + if topic_arn not in self.topics: + raise SNSNotFoundError('Topic does not exist') + + policy = self.topics[topic_arn]._policy_json + statement = next((statement for statement in policy['Statement'] if statement['Sid'] == label), None) + + if statement: + raise SNSInvalidParameter('Statement already exists') + + if any(action_name not in VALID_POLICY_ACTIONS for action_name in action_names): + raise SNSInvalidParameter('Policy statement action out of service scope!') + + principals = ['arn:aws:iam::{}:root'.format(account_id) for account_id in aws_account_ids] + actions = ['SNS:{}'.format(action_name) for action_name in action_names] + + statement = { + 'Sid': label, + 'Effect': 'Allow', + 'Principal': { + 'AWS': principals[0] if len(principals) == 1 else principals + }, + 'Action': actions[0] if len(actions) == 1 else actions, + 'Resource': topic_arn + } + + self.topics[topic_arn]._policy_json['Statement'].append(statement) + + def remove_permission(self, topic_arn, label): + if topic_arn not in self.topics: + raise SNSNotFoundError('Topic does not exist') + + statements = self.topics[topic_arn]._policy_json['Statement'] + statements = [statement for statement in statements if statement['Sid'] != label] + + self.topics[topic_arn]._policy_json['Statement'] = statements + def list_tags_for_resource(self, resource_arn): if resource_arn not in self.topics: raise ResourceNotFoundError @@ -542,35 +617,6 @@ for region in Session().get_available_regions('sns'): sns_backends[region] = SNSBackend(region) -DEFAULT_TOPIC_POLICY = { - "Version": "2008-10-17", - "Id": "us-east-1/698519295917/test__default_policy_ID", - "Statement": [{ - "Effect": "Allow", - "Sid": "us-east-1/698519295917/test__default_statement_ID", - "Principal": { - "AWS": "*" - }, - "Action": [ - "SNS:GetTopicAttributes", - "SNS:SetTopicAttributes", - "SNS:AddPermission", - "SNS:RemovePermission", - "SNS:DeleteTopic", - "SNS:Subscribe", - "SNS:ListSubscriptionsByTopic", - "SNS:Publish", - "SNS:Receive", - ], - "Resource": "arn:aws:sns:us-east-1:698519295917:test", - "Condition": { - "StringLike": { - "AWS:SourceArn": "arn:aws:*:*:698519295917:*" - } - } - }] -} - DEFAULT_EFFECTIVE_DELIVERY_POLICY = { 'http': { 'disableSubscriptionOverrides': False, @@ -585,3 +631,16 @@ DEFAULT_EFFECTIVE_DELIVERY_POLICY = { } } } + + +VALID_POLICY_ACTIONS = [ + 'GetTopicAttributes', + 'SetTopicAttributes', + 'AddPermission', + 'RemovePermission', + 'DeleteTopic', + 'Subscribe', + 'ListSubscriptionsByTopic', + 'Publish', + 'Receive' +] diff --git a/moto/sns/responses.py b/moto/sns/responses.py index c315e2a87..ced2d68a1 100644 --- a/moto/sns/responses.py +++ b/moto/sns/responses.py @@ -639,34 +639,21 @@ class SNSResponse(BaseResponse): return template.render() def add_permission(self): - arn = self._get_param('TopicArn') + topic_arn = self._get_param('TopicArn') label = self._get_param('Label') - accounts = self._get_multi_param('AWSAccountId.member.') - action = self._get_multi_param('ActionName.member.') + aws_account_ids = self._get_multi_param('AWSAccountId.member.') + action_names = self._get_multi_param('ActionName.member.') - if arn not in self.backend.topics: - error_response = self._error('NotFound', 'Topic does not exist') - return error_response, dict(status=404) - - key = (arn, label) - self.backend.permissions[key] = {'accounts': accounts, 'action': action} + self.backend.add_permission(topic_arn, label, aws_account_ids, action_names) template = self.response_template(ADD_PERMISSION_TEMPLATE) return template.render() def remove_permission(self): - arn = self._get_param('TopicArn') + topic_arn = self._get_param('TopicArn') label = self._get_param('Label') - if arn not in self.backend.topics: - error_response = self._error('NotFound', 'Topic does not exist') - return error_response, dict(status=404) - - try: - key = (arn, label) - del self.backend.permissions[key] - except KeyError: - pass + self.backend.remove_permission(topic_arn, label) template = self.response_template(DEL_PERMISSION_TEMPLATE) return template.render() diff --git a/tests/test_sns/test_topics.py b/tests/test_sns/test_topics.py index 1b039c51d..a7d9723cc 100644 --- a/tests/test_sns/test_topics.py +++ b/tests/test_sns/test_topics.py @@ -7,7 +7,7 @@ import sure # noqa from boto.exception import BotoServerError from moto import mock_sns_deprecated -from moto.sns.models import DEFAULT_TOPIC_POLICY, DEFAULT_EFFECTIVE_DELIVERY_POLICY, DEFAULT_PAGE_SIZE +from moto.sns.models import DEFAULT_EFFECTIVE_DELIVERY_POLICY, DEFAULT_PAGE_SIZE @mock_sns_deprecated @@ -76,7 +76,34 @@ def test_topic_attributes(): .format(conn.region.name) ) attributes["Owner"].should.equal(123456789012) - json.loads(attributes["Policy"]).should.equal(DEFAULT_TOPIC_POLICY) + json.loads(attributes["Policy"]).should.equal({ + "Version": "2008-10-17", + "Id": "__default_policy_ID", + "Statement": [{ + "Effect": "Allow", + "Sid": "__default_statement_ID", + "Principal": { + "AWS": "*" + }, + "Action": [ + "SNS:GetTopicAttributes", + "SNS:SetTopicAttributes", + "SNS:AddPermission", + "SNS:RemovePermission", + "SNS:DeleteTopic", + "SNS:Subscribe", + "SNS:ListSubscriptionsByTopic", + "SNS:Publish", + "SNS:Receive", + ], + "Resource": "arn:aws:sns:us-east-1:123456789012:some-topic", + "Condition": { + "StringEquals": { + "AWS:SourceOwner": "123456789012" + } + } + }] + }) attributes["DisplayName"].should.equal("") attributes["SubscriptionsPending"].should.equal(0) attributes["SubscriptionsConfirmed"].should.equal(0) @@ -89,11 +116,11 @@ def test_topic_attributes(): # i.e. unicode on Python 2 -- u"foobar" # and bytes on Python 3 -- b"foobar" if six.PY2: - policy = {b"foo": b"bar"} + policy = json.dumps({b"foo": b"bar"}) displayname = b"My display name" delivery = {b"http": {b"defaultHealthyRetryPolicy": {b"numRetries": 5}}} else: - policy = {u"foo": u"bar"} + policy = json.dumps({u"foo": u"bar"}) displayname = u"My display name" delivery = {u"http": {u"defaultHealthyRetryPolicy": {u"numRetries": 5}}} conn.set_topic_attributes(topic_arn, "Policy", policy) @@ -102,7 +129,7 @@ def test_topic_attributes(): attributes = conn.get_topic_attributes(topic_arn)['GetTopicAttributesResponse'][ 'GetTopicAttributesResult']['Attributes'] - attributes["Policy"].should.equal("{'foo': 'bar'}") + attributes["Policy"].should.equal('{"foo": "bar"}') attributes["DisplayName"].should.equal("My display name") attributes["DeliveryPolicy"].should.equal( "{'http': {'defaultHealthyRetryPolicy': {'numRetries': 5}}}") diff --git a/tests/test_sns/test_topics_boto3.py b/tests/test_sns/test_topics_boto3.py index 05c8f74b4..de7bb10cc 100644 --- a/tests/test_sns/test_topics_boto3.py +++ b/tests/test_sns/test_topics_boto3.py @@ -7,7 +7,7 @@ import sure # noqa from botocore.exceptions import ClientError from moto import mock_sns -from moto.sns.models import DEFAULT_TOPIC_POLICY, DEFAULT_EFFECTIVE_DELIVERY_POLICY, DEFAULT_PAGE_SIZE +from moto.sns.models import DEFAULT_EFFECTIVE_DELIVERY_POLICY, DEFAULT_PAGE_SIZE @mock_sns @@ -156,7 +156,34 @@ def test_topic_attributes(): .format(conn._client_config.region_name) ) attributes["Owner"].should.equal('123456789012') - json.loads(attributes["Policy"]).should.equal(DEFAULT_TOPIC_POLICY) + json.loads(attributes["Policy"]).should.equal({ + "Version": "2008-10-17", + "Id": "__default_policy_ID", + "Statement": [{ + "Effect": "Allow", + "Sid": "__default_statement_ID", + "Principal": { + "AWS": "*" + }, + "Action": [ + "SNS:GetTopicAttributes", + "SNS:SetTopicAttributes", + "SNS:AddPermission", + "SNS:RemovePermission", + "SNS:DeleteTopic", + "SNS:Subscribe", + "SNS:ListSubscriptionsByTopic", + "SNS:Publish", + "SNS:Receive", + ], + "Resource": "arn:aws:sns:us-east-1:123456789012:some-topic", + "Condition": { + "StringEquals": { + "AWS:SourceOwner": "123456789012" + } + } + }] + }) attributes["DisplayName"].should.equal("") attributes["SubscriptionsPending"].should.equal('0') attributes["SubscriptionsConfirmed"].should.equal('0') @@ -217,18 +244,190 @@ def test_topic_paging(): @mock_sns def test_add_remove_permissions(): - conn = boto3.client('sns', region_name='us-east-1') - response = conn.create_topic(Name='testpermissions') + client = boto3.client('sns', region_name='us-east-1') + topic_arn = client.create_topic(Name='test-permissions')['TopicArn'] - conn.add_permission( - TopicArn=response['TopicArn'], - Label='Test1234', + client.add_permission( + TopicArn=topic_arn, + Label='test', + AWSAccountId=['999999999999'], + ActionName=['Publish'] + ) + + response = client.get_topic_attributes(TopicArn=topic_arn) + json.loads(response['Attributes']['Policy']).should.equal({ + 'Version': '2008-10-17', + 'Id': '__default_policy_ID', + 'Statement': [ + { + 'Effect': 'Allow', + 'Sid': '__default_statement_ID', + 'Principal': { + 'AWS': '*' + }, + 'Action': [ + 'SNS:GetTopicAttributes', + 'SNS:SetTopicAttributes', + 'SNS:AddPermission', + 'SNS:RemovePermission', + 'SNS:DeleteTopic', + 'SNS:Subscribe', + 'SNS:ListSubscriptionsByTopic', + 'SNS:Publish', + 'SNS:Receive', + ], + 'Resource': 'arn:aws:sns:us-east-1:123456789012:test-permissions', + 'Condition': { + 'StringEquals': { + 'AWS:SourceOwner': '123456789012' + } + } + }, + { + 'Sid': 'test', + 'Effect': 'Allow', + 'Principal': { + 'AWS': 'arn:aws:iam::999999999999:root' + }, + 'Action': 'SNS:Publish', + 'Resource': 'arn:aws:sns:us-east-1:123456789012:test-permissions' + } + ] + }) + + client.remove_permission( + TopicArn=topic_arn, + Label='test' + ) + + response = client.get_topic_attributes(TopicArn=topic_arn) + json.loads(response['Attributes']['Policy']).should.equal({ + 'Version': '2008-10-17', + 'Id': '__default_policy_ID', + 'Statement': [ + { + 'Effect': 'Allow', + 'Sid': '__default_statement_ID', + 'Principal': { + 'AWS': '*' + }, + 'Action': [ + 'SNS:GetTopicAttributes', + 'SNS:SetTopicAttributes', + 'SNS:AddPermission', + 'SNS:RemovePermission', + 'SNS:DeleteTopic', + 'SNS:Subscribe', + 'SNS:ListSubscriptionsByTopic', + 'SNS:Publish', + 'SNS:Receive', + ], + 'Resource': 'arn:aws:sns:us-east-1:123456789012:test-permissions', + 'Condition': { + 'StringEquals': { + 'AWS:SourceOwner': '123456789012' + } + } + } + ] + }) + + client.add_permission( + TopicArn=topic_arn, + Label='test', + AWSAccountId=[ + '888888888888', + '999999999999' + ], + ActionName=[ + 'Publish', + 'Subscribe' + ] + ) + + response = client.get_topic_attributes(TopicArn=topic_arn) + json.loads(response['Attributes']['Policy'])['Statement'][1].should.equal({ + 'Sid': 'test', + 'Effect': 'Allow', + 'Principal': { + 'AWS': [ + 'arn:aws:iam::888888888888:root', + 'arn:aws:iam::999999999999:root' + ] + }, + 'Action': [ + 'SNS:Publish', + 'SNS:Subscribe' + ], + 'Resource': 'arn:aws:sns:us-east-1:123456789012:test-permissions' + }) + + # deleting non existing permission should be successful + client.remove_permission( + TopicArn=topic_arn, + Label='non-existing' + ) + + +@mock_sns +def test_add_permission_errors(): + client = boto3.client('sns', region_name='us-east-1') + topic_arn = client.create_topic(Name='test-permissions')['TopicArn'] + client.add_permission( + TopicArn=topic_arn, + Label='test', + AWSAccountId=['999999999999'], + ActionName=['Publish'] + ) + + client.add_permission.when.called_with( + TopicArn=topic_arn, + Label='test', AWSAccountId=['999999999999'], ActionName=['AddPermission'] + ).should.throw( + ClientError, + 'Statement already exists' ) - conn.remove_permission( - TopicArn=response['TopicArn'], - Label='Test1234' + + client.add_permission.when.called_with( + TopicArn=topic_arn + '-not-existing', + Label='test-2', + AWSAccountId=['999999999999'], + ActionName=['AddPermission'] + ).should.throw( + ClientError, + 'Topic does not exist' + ) + + client.add_permission.when.called_with( + TopicArn=topic_arn, + Label='test-2', + AWSAccountId=['999999999999'], + ActionName=['NotExistingAction'] + ).should.throw( + ClientError, + 'Policy statement action out of service scope!' + ) + + +@mock_sns +def test_remove_permission_errors(): + client = boto3.client('sns', region_name='us-east-1') + topic_arn = client.create_topic(Name='test-permissions')['TopicArn'] + client.add_permission( + TopicArn=topic_arn, + Label='test', + AWSAccountId=['999999999999'], + ActionName=['Publish'] + ) + + client.remove_permission.when.called_with( + TopicArn=topic_arn + '-not-existing', + Label='test', + ).should.throw( + ClientError, + 'Topic does not exist' ) From 23978e644fae10756d97c446d6db4c7f7a5b4c04 Mon Sep 17 00:00:00 2001 From: gruebel Date: Sat, 26 Oct 2019 22:08:45 +0200 Subject: [PATCH 02/15] Refactor sqs.send_message_batch --- IMPLEMENTATION_COVERAGE.md | 6 ++-- moto/sqs/exceptions.py | 53 +++++++++++++++++++++++++++++ moto/sqs/models.py | 52 ++++++++++++++++++++++++++++ moto/sqs/responses.py | 70 +++++++++----------------------------- tests/test_sqs/test_sqs.py | 66 +++++++++++++++++++++++++++++++++-- 5 files changed, 187 insertions(+), 60 deletions(-) diff --git a/IMPLEMENTATION_COVERAGE.md b/IMPLEMENTATION_COVERAGE.md index 072173226..126d454c6 100644 --- a/IMPLEMENTATION_COVERAGE.md +++ b/IMPLEMENTATION_COVERAGE.md @@ -6065,7 +6065,7 @@ - [X] untag_resource ## sqs -65% implemented +75% implemented - [X] add_permission - [X] change_message_visibility - [ ] change_message_visibility_batch @@ -6076,13 +6076,13 @@ - [ ] get_queue_attributes - [ ] get_queue_url - [X] list_dead_letter_source_queues -- [ ] list_queue_tags +- [X] list_queue_tags - [X] list_queues - [X] purge_queue - [ ] receive_message - [X] remove_permission - [X] send_message -- [ ] send_message_batch +- [X] send_message_batch - [X] set_queue_attributes - [X] tag_queue - [X] untag_queue diff --git a/moto/sqs/exceptions.py b/moto/sqs/exceptions.py index 02f28b2d2..6eee5b843 100644 --- a/moto/sqs/exceptions.py +++ b/moto/sqs/exceptions.py @@ -33,3 +33,56 @@ class QueueAlreadyExists(RESTError): def __init__(self, message): super(QueueAlreadyExists, self).__init__( "QueueAlreadyExists", message) + + +class EmptyBatchRequest(RESTError): + code = 400 + + def __init__(self): + super(EmptyBatchRequest, self).__init__( + 'EmptyBatchRequest', + 'There should be at least one SendMessageBatchRequestEntry in the request.' + ) + + +class InvalidBatchEntryId(RESTError): + code = 400 + + def __init__(self): + super(InvalidBatchEntryId, self).__init__( + 'InvalidBatchEntryId', + 'A batch entry id can only contain alphanumeric characters, ' + 'hyphens and underscores. It can be at most 80 letters long.' + ) + + +class BatchRequestTooLong(RESTError): + code = 400 + + def __init__(self, length): + super(BatchRequestTooLong, self).__init__( + 'BatchRequestTooLong', + 'Batch requests cannot be longer than 262144 bytes. ' + 'You have sent {} bytes.'.format(length) + ) + + +class BatchEntryIdsNotDistinct(RESTError): + code = 400 + + def __init__(self, entry_id): + super(BatchEntryIdsNotDistinct, self).__init__( + 'BatchEntryIdsNotDistinct', + 'Id {} repeated.'.format(entry_id) + ) + + +class TooManyEntriesInBatchRequest(RESTError): + code = 400 + + def __init__(self, number): + super(TooManyEntriesInBatchRequest, self).__init__( + 'TooManyEntriesInBatchRequest', + 'Maximum number of entries per request are 10. ' + 'You have sent {}.'.format(number) + ) diff --git a/moto/sqs/models.py b/moto/sqs/models.py index eb237e437..9900846ac 100644 --- a/moto/sqs/models.py +++ b/moto/sqs/models.py @@ -20,11 +20,17 @@ from .exceptions import ( QueueDoesNotExist, QueueAlreadyExists, ReceiptHandleIsInvalid, + InvalidBatchEntryId, + BatchRequestTooLong, + BatchEntryIdsNotDistinct, + TooManyEntriesInBatchRequest ) DEFAULT_ACCOUNT_ID = 123456789012 DEFAULT_SENDER_ID = "AIDAIT2UOQQY3AUEKVGXU" +MAXIMUM_MESSAGE_LENGTH = 262144 # 256 KiB + TRANSPORT_TYPE_ENCODINGS = {'String': b'\x01', 'Binary': b'\x02', 'Number': b'\x01'} @@ -516,6 +522,49 @@ class SQSBackend(BaseBackend): return message + def send_message_batch(self, queue_name, entries): + self.get_queue(queue_name) + + if any(not re.match(r'^[\w-]{1,80}$', entry['Id']) for entry in entries.values()): + raise InvalidBatchEntryId() + + body_length = next( + (len(entry['MessageBody']) for entry in entries.values() if len(entry['MessageBody']) > MAXIMUM_MESSAGE_LENGTH), + False + ) + if body_length: + raise BatchRequestTooLong(body_length) + + duplicate_id = self._get_first_duplicate_id([entry['Id'] for entry in entries.values()]) + if duplicate_id: + raise BatchEntryIdsNotDistinct(duplicate_id) + + if len(entries) > 10: + raise TooManyEntriesInBatchRequest(len(entries)) + + messages = [] + for index, entry in entries.items(): + # Loop through looking for messages + message = self.send_message( + queue_name, + entry['MessageBody'], + message_attributes=entry['MessageAttributes'], + delay_seconds=entry['DelaySeconds'] + ) + message.user_id = entry['Id'] + + messages.append(message) + + return messages + + def _get_first_duplicate_id(self, ids): + unique_ids = set() + for id in ids: + if id in unique_ids: + return id + unique_ids.add(id) + return None + def receive_messages(self, queue_name, count, wait_seconds_timeout, visibility_timeout): """ Attempt to retrieve visible messages from a queue. @@ -677,6 +726,9 @@ class SQSBackend(BaseBackend): except KeyError: pass + def list_queue_tags(self, queue_name): + return self.get_queue(queue_name) + sqs_backends = {} for region in boto.sqs.regions(): diff --git a/moto/sqs/responses.py b/moto/sqs/responses.py index b6f717f3b..52b237235 100644 --- a/moto/sqs/responses.py +++ b/moto/sqs/responses.py @@ -11,6 +11,7 @@ from .exceptions import ( MessageAttributesInvalid, MessageNotInflight, ReceiptHandleIsInvalid, + EmptyBatchRequest ) MAXIMUM_VISIBILTY_TIMEOUT = 43200 @@ -237,72 +238,33 @@ class SQSResponse(BaseResponse): self.sqs_backend.get_queue(queue_name) if self.querystring.get('Entries'): - return self._error('AWS.SimpleQueueService.EmptyBatchRequest', - 'There should be at least one SendMessageBatchRequestEntry in the request.') + raise EmptyBatchRequest() entries = {} for key, value in self.querystring.items(): match = re.match(r'^SendMessageBatchRequestEntry\.(\d+)\.Id', key) if match: - entries[match.group(1)] = { + index = match.group(1) + + message_attributes = parse_message_attributes( + self.querystring, base='SendMessageBatchRequestEntry.{}.'.format(index)) + if type(message_attributes) == tuple: + return message_attributes[0], message_attributes[1] + + entries[index] = { 'Id': value[0], 'MessageBody': self.querystring.get( - 'SendMessageBatchRequestEntry.{}.MessageBody'.format(match.group(1)))[0] + 'SendMessageBatchRequestEntry.{}.MessageBody'.format(index))[0], + 'DelaySeconds': self.querystring.get( + 'SendMessageBatchRequestEntry.{}.DelaySeconds'.format(index), [None])[0], + 'MessageAttributes': message_attributes } - if any(not re.match(r'^[\w-]{1,80}$', entry['Id']) for entry in entries.values()): - return self._error('AWS.SimpleQueueService.InvalidBatchEntryId', - 'A batch entry id can only contain alphanumeric characters, ' - 'hyphens and underscores. It can be at most 80 letters long.') - - body_length = next( - (len(entry['MessageBody']) for entry in entries.values() if len(entry['MessageBody']) > MAXIMUM_MESSAGE_LENGTH), - False - ) - if body_length: - return self._error('AWS.SimpleQueueService.BatchRequestTooLong', - 'Batch requests cannot be longer than 262144 bytes. ' - 'You have sent {} bytes.'.format(body_length)) - - duplicate_id = self._get_first_duplicate_id([entry['Id'] for entry in entries.values()]) - if duplicate_id: - return self._error('AWS.SimpleQueueService.BatchEntryIdsNotDistinct', - 'Id {} repeated.'.format(duplicate_id)) - - if len(entries) > 10: - return self._error('AWS.SimpleQueueService.TooManyEntriesInBatchRequest', - 'Maximum number of entries per request are 10. ' - 'You have sent 11.') - - messages = [] - for index, entry in entries.items(): - # Loop through looking for messages - delay_key = 'SendMessageBatchRequestEntry.{0}.DelaySeconds'.format( - index) - delay_seconds = self.querystring.get(delay_key, [None])[0] - message = self.sqs_backend.send_message( - queue_name, entry['MessageBody'], delay_seconds=delay_seconds) - message.user_id = entry['Id'] - - message_attributes = parse_message_attributes( - self.querystring, base='SendMessageBatchRequestEntry.{0}.'.format(index)) - if type(message_attributes) == tuple: - return message_attributes[0], message_attributes[1] - message.message_attributes = message_attributes - - messages.append(message) + messages = self.sqs_backend.send_message_batch(queue_name, entries) template = self.response_template(SEND_MESSAGE_BATCH_RESPONSE) return template.render(messages=messages) - def _get_first_duplicate_id(self, ids): - unique_ids = set() - for id in ids: - if id in unique_ids: - return id - unique_ids.add(id) - return None - def delete_message(self): queue_name = self._get_queue_name() receipt_handle = self.querystring.get("ReceiptHandle")[0] @@ -441,7 +403,7 @@ class SQSResponse(BaseResponse): def list_queue_tags(self): queue_name = self._get_queue_name() - queue = self.sqs_backend.get_queue(queue_name) + queue = self.sqs_backend.list_queue_tags(queue_name) template = self.response_template(LIST_QUEUE_TAGS_RESPONSE) return template.render(tags=queue.tags) diff --git a/tests/test_sqs/test_sqs.py b/tests/test_sqs/test_sqs.py index 1ad2e1a80..1a9038aa5 100644 --- a/tests/test_sqs/test_sqs.py +++ b/tests/test_sqs/test_sqs.py @@ -883,10 +883,70 @@ def test_delete_message_after_visibility_timeout(): @mock_sqs -def test_send_message_batch_errors(): - client = boto3.client('sqs', region_name = 'us-east-1') +def test_send_message_batch(): + client = boto3.client('sqs', region_name='us-east-1') + response = client.create_queue(QueueName='test-queue') + queue_url = response['QueueUrl'] - response = client.create_queue(QueueName='test-queue-with-tags') + response = client.send_message_batch( + QueueUrl=queue_url, + Entries=[ + { + 'Id': 'id_1', + 'MessageBody': 'body_1', + 'DelaySeconds': 0, + 'MessageAttributes': { + 'attribute_name_1': { + 'StringValue': 'attribute_value_1', + 'DataType': 'String' + } + } + }, + { + 'Id': 'id_2', + 'MessageBody': 'body_2', + 'DelaySeconds': 0, + 'MessageAttributes': { + 'attribute_name_2': { + 'StringValue': '123', + 'DataType': 'Number' + } + } + } + ] + ) + + sorted([entry['Id'] for entry in response['Successful']]).should.equal([ + 'id_1', + 'id_2' + ]) + + response = client.receive_message( + QueueUrl=queue_url, + MaxNumberOfMessages=10 + ) + + response['Messages'][0]['Body'].should.equal('body_1') + response['Messages'][0]['MessageAttributes'].should.equal({ + 'attribute_name_1': { + 'StringValue': 'attribute_value_1', + 'DataType': 'String' + } + }) + response['Messages'][1]['Body'].should.equal('body_2') + response['Messages'][1]['MessageAttributes'].should.equal({ + 'attribute_name_2': { + 'StringValue': '123', + 'DataType': 'Number' + } + }) + + +@mock_sqs +def test_send_message_batch_errors(): + client = boto3.client('sqs', region_name='us-east-1') + + response = client.create_queue(QueueName='test-queue') queue_url = response['QueueUrl'] client.send_message_batch.when.called_with( From 6573f69087d6ca3229e72a37fa6f657074cddefc Mon Sep 17 00:00:00 2001 From: gruebel Date: Sat, 26 Oct 2019 22:26:48 +0200 Subject: [PATCH 03/15] Refactor sqs.get_queue_url --- IMPLEMENTATION_COVERAGE.md | 4 ++-- moto/sqs/models.py | 3 +++ moto/sqs/responses.py | 11 ++++------- tests/test_sqs/test_sqs.py | 30 +++++++++++++++++++++++++++--- 4 files changed, 36 insertions(+), 12 deletions(-) diff --git a/IMPLEMENTATION_COVERAGE.md b/IMPLEMENTATION_COVERAGE.md index 126d454c6..40daef880 100644 --- a/IMPLEMENTATION_COVERAGE.md +++ b/IMPLEMENTATION_COVERAGE.md @@ -6065,7 +6065,7 @@ - [X] untag_resource ## sqs -75% implemented +80% implemented - [X] add_permission - [X] change_message_visibility - [ ] change_message_visibility_batch @@ -6074,7 +6074,7 @@ - [ ] delete_message_batch - [X] delete_queue - [ ] get_queue_attributes -- [ ] get_queue_url +- [X] get_queue_url - [X] list_dead_letter_source_queues - [X] list_queue_tags - [X] list_queues diff --git a/moto/sqs/models.py b/moto/sqs/models.py index 9900846ac..64b376d60 100644 --- a/moto/sqs/models.py +++ b/moto/sqs/models.py @@ -466,6 +466,9 @@ class SQSBackend(BaseBackend): return queue + def get_queue_url(self, queue_name): + return self.get_queue(queue_name) + def list_queues(self, queue_name_prefix): re_str = '.*' if queue_name_prefix: diff --git a/moto/sqs/responses.py b/moto/sqs/responses.py index 52b237235..2997f924f 100644 --- a/moto/sqs/responses.py +++ b/moto/sqs/responses.py @@ -90,13 +90,10 @@ class SQSResponse(BaseResponse): request_url = urlparse(self.uri) queue_name = self._get_param("QueueName") - queue = self.sqs_backend.get_queue(queue_name) + queue = self.sqs_backend.get_queue_url(queue_name) - if queue: - template = self.response_template(GET_QUEUE_URL_RESPONSE) - return template.render(queue=queue, request_url=request_url) - else: - return "", dict(status=404) + template = self.response_template(GET_QUEUE_URL_RESPONSE) + return template.render(queue_url=queue.url(request_url)) def list_queues(self): request_url = urlparse(self.uri) @@ -420,7 +417,7 @@ CREATE_QUEUE_RESPONSE = """ GET_QUEUE_URL_RESPONSE = """ - {{ queue.url(request_url) }} + {{ queue_url }} diff --git a/tests/test_sqs/test_sqs.py b/tests/test_sqs/test_sqs.py index 1a9038aa5..7a067c316 100644 --- a/tests/test_sqs/test_sqs.py +++ b/tests/test_sqs/test_sqs.py @@ -144,18 +144,42 @@ def test_create_queue_kms(): def test_create_queue_with_tags(): client = boto3.client('sqs', region_name='us-east-1') response = client.create_queue( - QueueName = 'test-queue-with-tags', - tags = { + QueueName='test-queue-with-tags', + tags={ 'tag_key_1': 'tag_value_1' } ) queue_url = response['QueueUrl'] - client.list_queue_tags(QueueUrl = queue_url)['Tags'].should.equal({ + client.list_queue_tags(QueueUrl=queue_url)['Tags'].should.equal({ 'tag_key_1': 'tag_value_1' }) +@mock_sqs +def test_get_queue_url(): + client = boto3.client('sqs', region_name='us-east-1') + client.create_queue(QueueName='test-queue') + + response = client.get_queue_url(QueueName='test-queue') + + response['QueueUrl'].should.equal( + 'https://queue.amazonaws.com/123456789012/test-queue' + ) + + +@mock_sqs +def test_get_queue_url_errors(): + client = boto3.client('sqs', region_name='us-east-1') + + client.get_queue_url.when.called_with( + QueueName='non-existing-queue' + ).should.throw( + ClientError, + 'The specified queue does not exist for this wsdl version.' + ) + + @mock_sqs def test_get_nonexistent_queue(): sqs = boto3.resource('sqs', region_name='us-east-1') From 77bc97c8da7f652848de970d7be2d2a4195f6a4d Mon Sep 17 00:00:00 2001 From: Chih-Hsuan Yen Date: Sun, 27 Oct 2019 16:58:03 +0800 Subject: [PATCH 04/15] Fix ECR models for Python 3.8 Before this fix, using moto.ecr with Python 3.8 results in the following error: RuntimeError: dictionary keys changed during iteration --- moto/ecr/models.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/moto/ecr/models.py b/moto/ecr/models.py index b03f25dee..f6419269e 100644 --- a/moto/ecr/models.py +++ b/moto/ecr/models.py @@ -2,7 +2,6 @@ from __future__ import unicode_literals import hashlib import re -from copy import copy from datetime import datetime from random import random @@ -27,11 +26,12 @@ class BaseObject(BaseModel): return ''.join(words) def gen_response_object(self): - response_object = copy(self.__dict__) - for key, value in response_object.items(): + response_object = dict() + for key, value in self.__dict__.items(): if '_' in key: response_object[self.camelCase(key)] = value - del response_object[key] + else: + response_object[key] = value return response_object @property From c3cb411c07aded217979011dd8492f5988260294 Mon Sep 17 00:00:00 2001 From: gruebel Date: Sun, 27 Oct 2019 12:13:33 +0100 Subject: [PATCH 05/15] Refactor sqs.get_queue_attributes & add AttributeNames handling --- IMPLEMENTATION_COVERAGE.md | 4 +- moto/core/responses.py | 2 +- moto/sqs/exceptions.py | 10 ++++ moto/sqs/models.py | 42 +++++++++++++---- moto/sqs/responses.py | 14 ++++-- tests/test_sqs/test_sqs.py | 93 ++++++++++++++++++++++++++++++++++++++ 6 files changed, 149 insertions(+), 16 deletions(-) diff --git a/IMPLEMENTATION_COVERAGE.md b/IMPLEMENTATION_COVERAGE.md index 40daef880..691099e7f 100644 --- a/IMPLEMENTATION_COVERAGE.md +++ b/IMPLEMENTATION_COVERAGE.md @@ -6065,7 +6065,7 @@ - [X] untag_resource ## sqs -80% implemented +85% implemented - [X] add_permission - [X] change_message_visibility - [ ] change_message_visibility_batch @@ -6073,7 +6073,7 @@ - [X] delete_message - [ ] delete_message_batch - [X] delete_queue -- [ ] get_queue_attributes +- [X] get_queue_attributes - [X] get_queue_url - [X] list_dead_letter_source_queues - [X] list_queue_tags diff --git a/moto/core/responses.py b/moto/core/responses.py index b60f10a20..213fa278c 100644 --- a/moto/core/responses.py +++ b/moto/core/responses.py @@ -454,7 +454,7 @@ class BaseResponse(_TemplateEnvironmentMixin, ActionAuthenticatorMixin): index = 1 while True: value_dict = self._get_multi_param_helper(prefix + str(index)) - if not value_dict: + if not value_dict and value_dict != '': break values.append(value_dict) diff --git a/moto/sqs/exceptions.py b/moto/sqs/exceptions.py index 6eee5b843..44ed64611 100644 --- a/moto/sqs/exceptions.py +++ b/moto/sqs/exceptions.py @@ -86,3 +86,13 @@ class TooManyEntriesInBatchRequest(RESTError): 'Maximum number of entries per request are 10. ' 'You have sent {}.'.format(number) ) + + +class InvalidAttributeName(RESTError): + code = 400 + + def __init__(self, attribute_name): + super(InvalidAttributeName, self).__init__( + 'InvalidAttributeName', + 'Unknown Attribute {}.'.format(attribute_name) + ) diff --git a/moto/sqs/models.py b/moto/sqs/models.py index 64b376d60..0df5fb7c4 100644 --- a/moto/sqs/models.py +++ b/moto/sqs/models.py @@ -23,7 +23,8 @@ from .exceptions import ( InvalidBatchEntryId, BatchRequestTooLong, BatchEntryIdsNotDistinct, - TooManyEntriesInBatchRequest + TooManyEntriesInBatchRequest, + InvalidAttributeName ) DEFAULT_ACCOUNT_ID = 123456789012 @@ -161,7 +162,7 @@ class Message(BaseModel): class Queue(BaseModel): - base_attributes = ['ApproximateNumberOfMessages', + BASE_ATTRIBUTES = ['ApproximateNumberOfMessages', 'ApproximateNumberOfMessagesDelayed', 'ApproximateNumberOfMessagesNotVisible', 'CreatedTimestamp', @@ -172,9 +173,9 @@ class Queue(BaseModel): 'QueueArn', 'ReceiveMessageWaitTimeSeconds', 'VisibilityTimeout'] - fifo_attributes = ['FifoQueue', + FIFO_ATTRIBUTES = ['FifoQueue', 'ContentBasedDeduplication'] - kms_attributes = ['KmsDataKeyReusePeriodSeconds', + KMS_ATTRIBUTES = ['KmsDataKeyReusePeriodSeconds', 'KmsMasterKeyId'] ALLOWED_PERMISSIONS = ('*', 'ChangeMessageVisibility', 'DeleteMessage', 'GetQueueAttributes', 'GetQueueUrl', @@ -191,8 +192,9 @@ class Queue(BaseModel): now = unix_time() self.created_timestamp = now - self.queue_arn = 'arn:aws:sqs:{0}:123456789012:{1}'.format(self.region, - self.name) + self.queue_arn = 'arn:aws:sqs:{0}:{1}:{2}'.format(self.region, + DEFAULT_ACCOUNT_ID, + self.name) self.dead_letter_queue = None self.lambda_event_source_mappings = {} @@ -336,17 +338,17 @@ class Queue(BaseModel): def attributes(self): result = {} - for attribute in self.base_attributes: + for attribute in self.BASE_ATTRIBUTES: attr = getattr(self, camelcase_to_underscores(attribute)) result[attribute] = attr if self.fifo_queue: - for attribute in self.fifo_attributes: + for attribute in self.FIFO_ATTRIBUTES: attr = getattr(self, camelcase_to_underscores(attribute)) result[attribute] = attr if self.kms_master_key_id: - for attribute in self.kms_attributes: + for attribute in self.KMS_ATTRIBUTES: attr = getattr(self, camelcase_to_underscores(attribute)) result[attribute] = attr @@ -491,6 +493,28 @@ class SQSBackend(BaseBackend): return self.queues.pop(queue_name) return False + def get_queue_attributes(self, queue_name, attribute_names): + queue = self.get_queue(queue_name) + + if not len(attribute_names): + attribute_names.append('All') + + valid_names = ['All'] + queue.BASE_ATTRIBUTES + queue.FIFO_ATTRIBUTES + queue.KMS_ATTRIBUTES + invalid_name = next((name for name in attribute_names if name not in valid_names), None) + + if invalid_name or invalid_name == '': + raise InvalidAttributeName(invalid_name) + + attributes = {} + + if 'All' in attribute_names: + attributes = queue.attributes + else: + for name in (name for name in attribute_names if name in queue.attributes): + attributes[name] = queue.attributes.get(name) + + return attributes + def set_queue_attributes(self, queue_name, attributes): queue = self.get_queue(queue_name) queue._set_attributes(attributes) diff --git a/moto/sqs/responses.py b/moto/sqs/responses.py index 2997f924f..3d5ee1fea 100644 --- a/moto/sqs/responses.py +++ b/moto/sqs/responses.py @@ -11,7 +11,8 @@ from .exceptions import ( MessageAttributesInvalid, MessageNotInflight, ReceiptHandleIsInvalid, - EmptyBatchRequest + EmptyBatchRequest, + InvalidAttributeName ) MAXIMUM_VISIBILTY_TIMEOUT = 43200 @@ -169,10 +170,15 @@ class SQSResponse(BaseResponse): def get_queue_attributes(self): queue_name = self._get_queue_name() - queue = self.sqs_backend.get_queue(queue_name) + if self.querystring.get('AttributeNames'): + raise InvalidAttributeName('') + + attribute_names = self._get_multi_param('AttributeName') + + attributes = self.sqs_backend.get_queue_attributes(queue_name, attribute_names) template = self.response_template(GET_QUEUE_ATTRIBUTES_RESPONSE) - return template.render(queue=queue) + return template.render(attributes=attributes) def set_queue_attributes(self): # TODO validate self.get_param('QueueUrl') @@ -443,7 +449,7 @@ DELETE_QUEUE_RESPONSE = """ GET_QUEUE_ATTRIBUTES_RESPONSE = """ - {% for key, value in queue.attributes.items() %} + {% for key, value in attributes.items() %} {{ key }} {{ value }} diff --git a/tests/test_sqs/test_sqs.py b/tests/test_sqs/test_sqs.py index 7a067c316..42a540d5b 100644 --- a/tests/test_sqs/test_sqs.py +++ b/tests/test_sqs/test_sqs.py @@ -5,6 +5,7 @@ import os import boto import boto3 import botocore.exceptions +import six from botocore.exceptions import ClientError from boto.exception import SQSError from boto.sqs.message import RawMessage, Message @@ -365,6 +366,98 @@ def test_delete_queue(): queue.delete() +@mock_sqs +def test_get_queue_attributes(): + client = boto3.client('sqs', region_name='us-east-1') + response = client.create_queue(QueueName='test-queue') + queue_url = response['QueueUrl'] + + response = client.get_queue_attributes(QueueUrl=queue_url) + + response['Attributes']['ApproximateNumberOfMessages'].should.equal('0') + response['Attributes']['ApproximateNumberOfMessagesDelayed'].should.equal('0') + response['Attributes']['ApproximateNumberOfMessagesNotVisible'].should.equal('0') + response['Attributes']['CreatedTimestamp'].should.be.a(six.string_types) + response['Attributes']['DelaySeconds'].should.equal('0') + response['Attributes']['LastModifiedTimestamp'].should.be.a(six.string_types) + response['Attributes']['MaximumMessageSize'].should.equal('65536') + response['Attributes']['MessageRetentionPeriod'].should.equal('345600') + response['Attributes']['QueueArn'].should.equal('arn:aws:sqs:us-east-1:123456789012:test-queue') + response['Attributes']['ReceiveMessageWaitTimeSeconds'].should.equal('0') + response['Attributes']['VisibilityTimeout'].should.equal('30') + + response = client.get_queue_attributes( + QueueUrl=queue_url, + AttributeNames=[ + 'ApproximateNumberOfMessages', + 'MaximumMessageSize', + 'QueueArn', + 'VisibilityTimeout' + ] + ) + + response['Attributes'].should.equal({ + 'ApproximateNumberOfMessages': '0', + 'MaximumMessageSize': '65536', + 'QueueArn': 'arn:aws:sqs:us-east-1:123456789012:test-queue', + 'VisibilityTimeout': '30' + }) + + # should not return any attributes, if it was not set before + response = client.get_queue_attributes( + QueueUrl=queue_url, + AttributeNames=[ + 'KmsMasterKeyId' + ] + ) + + response.should_not.have.key('Attributes') + + +@mock_sqs +def test_get_queue_attributes_errors(): + client = boto3.client('sqs', region_name='us-east-1') + response = client.create_queue(QueueName='test-queue') + queue_url = response['QueueUrl'] + + client.get_queue_attributes.when.called_with( + QueueUrl=queue_url + '-non-existing' + ).should.throw( + ClientError, + 'The specified queue does not exist for this wsdl version.' + ) + + client.get_queue_attributes.when.called_with( + QueueUrl=queue_url, + AttributeNames=[ + 'QueueArn', + 'not-existing', + 'VisibilityTimeout' + ] + ).should.throw( + ClientError, + 'Unknown Attribute not-existing.' + ) + + client.get_queue_attributes.when.called_with( + QueueUrl=queue_url, + AttributeNames=[ + '' + ] + ).should.throw( + ClientError, + 'Unknown Attribute .' + ) + + client.get_queue_attributes.when.called_with( + QueueUrl = queue_url, + AttributeNames = [] + ).should.throw( + ClientError, + 'Unknown Attribute .' + ) + + @mock_sqs def test_set_queue_attribute(): sqs = boto3.resource('sqs', region_name='us-east-1') From f1ebe8d9467e3b8a8938820061dc4f152452a3c4 Mon Sep 17 00:00:00 2001 From: Chih-Hsuan Yen Date: Sun, 27 Oct 2019 19:38:16 +0800 Subject: [PATCH 06/15] Enable Travis CI tests for Pytho 3.8 --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 77dd2ae55..3f73af5aa 100644 --- a/.travis.yml +++ b/.travis.yml @@ -7,6 +7,7 @@ python: - 2.7 - 3.6 - 3.7 +- 3.8 env: - TEST_SERVER_MODE=false - TEST_SERVER_MODE=true From 51117c948a4203f160ab8689ee12e32a753138e1 Mon Sep 17 00:00:00 2001 From: gruebel Date: Sun, 27 Oct 2019 12:46:59 +0100 Subject: [PATCH 07/15] Add error handling to sqs.delete_message --- moto/sqs/exceptions.py | 11 ++++++++--- moto/sqs/models.py | 4 ++++ moto/sqs/responses.py | 2 +- tests/test_sqs/test_sqs.py | 30 ++++++++++++++++++++++++++++++ 4 files changed, 43 insertions(+), 4 deletions(-) diff --git a/moto/sqs/exceptions.py b/moto/sqs/exceptions.py index 44ed64611..68c4abaae 100644 --- a/moto/sqs/exceptions.py +++ b/moto/sqs/exceptions.py @@ -7,9 +7,14 @@ class MessageNotInflight(Exception): status_code = 400 -class ReceiptHandleIsInvalid(Exception): - description = "The receipt handle provided is not valid." - status_code = 400 +class ReceiptHandleIsInvalid(RESTError): + code = 400 + + def __init__(self): + super(ReceiptHandleIsInvalid, self).__init__( + 'ReceiptHandleIsInvalid', + 'The input receipt handle is invalid.' + ) class MessageAttributesInvalid(Exception): diff --git a/moto/sqs/models.py b/moto/sqs/models.py index 0df5fb7c4..8d02fe529 100644 --- a/moto/sqs/models.py +++ b/moto/sqs/models.py @@ -669,6 +669,10 @@ class SQSBackend(BaseBackend): def delete_message(self, queue_name, receipt_handle): queue = self.get_queue(queue_name) + + if not any(message.receipt_handle == receipt_handle for message in queue._messages): + raise ReceiptHandleIsInvalid() + new_messages = [] for message in queue._messages: # Only delete message if it is not visible and the reciept_handle diff --git a/moto/sqs/responses.py b/moto/sqs/responses.py index 3d5ee1fea..e6982b795 100644 --- a/moto/sqs/responses.py +++ b/moto/sqs/responses.py @@ -118,7 +118,7 @@ class SQSResponse(BaseResponse): receipt_handle=receipt_handle, visibility_timeout=visibility_timeout ) - except (ReceiptHandleIsInvalid, MessageNotInflight) as e: + except MessageNotInflight as e: return "Invalid request: {0}".format(e.description), dict(status=e.status_code) template = self.response_template(CHANGE_MESSAGE_VISIBILITY_RESPONSE) diff --git a/tests/test_sqs/test_sqs.py b/tests/test_sqs/test_sqs.py index 42a540d5b..2db457f72 100644 --- a/tests/test_sqs/test_sqs.py +++ b/tests/test_sqs/test_sqs.py @@ -999,6 +999,36 @@ def test_delete_message_after_visibility_timeout(): assert new_queue.count() == 0 +@mock_sqs +def test_delete_message_errors(): + client = boto3.client('sqs', region_name='us-east-1') + response = client.create_queue(QueueName='test-queue') + queue_url = response['QueueUrl'] + client.send_message( + QueueUrl=queue_url, + MessageBody='body' + ) + response = client.receive_message( + QueueUrl=queue_url + ) + receipt_handle = response['Messages'][0]['ReceiptHandle'] + + client.delete_message.when.called_with( + QueueUrl=queue_url + '-not-existing', + ReceiptHandle=receipt_handle + ).should.throw( + ClientError, + 'The specified queue does not exist for this wsdl version.' + ) + + client.delete_message.when.called_with( + QueueUrl=queue_url, + ReceiptHandle='not-existing' + ).should.throw( + ClientError, + 'The input receipt handle is invalid.' + ) + @mock_sqs def test_send_message_batch(): client = boto3.client('sqs', region_name='us-east-1') From 61698f955e69c48b425c2273ceb17865176e55ab Mon Sep 17 00:00:00 2001 From: Chih-Hsuan Yen Date: Sun, 27 Oct 2019 20:00:20 +0800 Subject: [PATCH 08/15] Use new flake8 for Python 3.8 compatibility With the current flake8, which depends on pyflakes<1.7.0,>=1.5.0, linting fails with: AttributeError: 'FlakesChecker' object has no attribute 'CONSTANT' The issue is fixed in pyflakes 2.1.0 [1]. Let's just use the latest flake8. [1] https://github.com/PyCQA/pyflakes/commit/7c74ab0ddccb60a9154c2af5aa4ee4581ccada4a --- requirements-dev.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements-dev.txt b/requirements-dev.txt index 1dd8ef1f8..c5fba292d 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -3,7 +3,7 @@ mock nose sure==1.4.11 coverage -flake8==3.5.0 +flake8==3.7.8 freezegun flask boto>=2.45.0 From 18173a59517e4f57e15a1312b032ec95f476f143 Mon Sep 17 00:00:00 2001 From: Chih-Hsuan Yen Date: Sun, 27 Oct 2019 20:41:22 +0800 Subject: [PATCH 09/15] Replace `# flake8: noqa` with `# noqa` The former syntax is actually a mis-use and rejected since Flake8 3.6 [1]. [1] https://gitlab.com/pycqa/flake8/merge_requests/219 --- moto/__init__.py | 98 +++++++++++++++++------------------ moto/compat.py | 4 +- moto/core/__init__.py | 2 +- moto/dynamodb2/comparisons.py | 12 ++--- moto/swf/models/__init__.py | 18 +++---- scripts/scaffold.py | 2 +- tests/test_ec2/test_vpcs.py | 2 +- 7 files changed, 69 insertions(+), 69 deletions(-) diff --git a/moto/__init__.py b/moto/__init__.py index 8e915933a..36d294657 100644 --- a/moto/__init__.py +++ b/moto/__init__.py @@ -5,55 +5,55 @@ import logging __title__ = 'moto' __version__ = '1.3.14.dev' -from .acm import mock_acm # flake8: noqa -from .apigateway import mock_apigateway, mock_apigateway_deprecated # flake8: noqa -from .athena import mock_athena # flake8: noqa -from .autoscaling import mock_autoscaling, mock_autoscaling_deprecated # flake8: noqa -from .awslambda import mock_lambda, mock_lambda_deprecated # flake8: noqa -from .cloudformation import mock_cloudformation, mock_cloudformation_deprecated # flake8: noqa -from .cloudwatch import mock_cloudwatch, mock_cloudwatch_deprecated # flake8: noqa -from .cognitoidentity import mock_cognitoidentity, mock_cognitoidentity_deprecated # flake8: noqa -from .cognitoidp import mock_cognitoidp, mock_cognitoidp_deprecated # flake8: noqa -from .config import mock_config # flake8: noqa -from .datapipeline import mock_datapipeline, mock_datapipeline_deprecated # flake8: noqa -from .dynamodb import mock_dynamodb, mock_dynamodb_deprecated # flake8: noqa -from .dynamodb2 import mock_dynamodb2, mock_dynamodb2_deprecated # flake8: noqa -from .dynamodbstreams import mock_dynamodbstreams # flake8: noqa -from .ec2 import mock_ec2, mock_ec2_deprecated # flake8: noqa -from .ecr import mock_ecr, mock_ecr_deprecated # flake8: noqa -from .ecs import mock_ecs, mock_ecs_deprecated # flake8: noqa -from .elb import mock_elb, mock_elb_deprecated # flake8: noqa -from .elbv2 import mock_elbv2 # flake8: noqa -from .emr import mock_emr, mock_emr_deprecated # flake8: noqa -from .events import mock_events # flake8: noqa -from .glacier import mock_glacier, mock_glacier_deprecated # flake8: noqa -from .glue import mock_glue # flake8: noqa -from .iam import mock_iam, mock_iam_deprecated # flake8: noqa -from .kinesis import mock_kinesis, mock_kinesis_deprecated # flake8: noqa -from .kms import mock_kms, mock_kms_deprecated # flake8: noqa -from .organizations import mock_organizations # flake8: noqa -from .opsworks import mock_opsworks, mock_opsworks_deprecated # flake8: noqa -from .polly import mock_polly # flake8: noqa -from .rds import mock_rds, mock_rds_deprecated # flake8: noqa -from .rds2 import mock_rds2, mock_rds2_deprecated # flake8: noqa -from .redshift import mock_redshift, mock_redshift_deprecated # flake8: noqa -from .resourcegroups import mock_resourcegroups # flake8: noqa -from .s3 import mock_s3, mock_s3_deprecated # flake8: noqa -from .ses import mock_ses, mock_ses_deprecated # flake8: noqa -from .secretsmanager import mock_secretsmanager # flake8: noqa -from .sns import mock_sns, mock_sns_deprecated # flake8: noqa -from .sqs import mock_sqs, mock_sqs_deprecated # flake8: noqa -from .stepfunctions import mock_stepfunctions # flake8: noqa -from .sts import mock_sts, mock_sts_deprecated # flake8: noqa -from .ssm import mock_ssm # flake8: noqa -from .route53 import mock_route53, mock_route53_deprecated # flake8: noqa -from .swf import mock_swf, mock_swf_deprecated # flake8: noqa -from .xray import mock_xray, mock_xray_client, XRaySegment # flake8: noqa -from .logs import mock_logs, mock_logs_deprecated # flake8: noqa -from .batch import mock_batch # flake8: noqa -from .resourcegroupstaggingapi import mock_resourcegroupstaggingapi # flake8: noqa -from .iot import mock_iot # flake8: noqa -from .iotdata import mock_iotdata # flake8: noqa +from .acm import mock_acm # noqa +from .apigateway import mock_apigateway, mock_apigateway_deprecated # noqa +from .athena import mock_athena # noqa +from .autoscaling import mock_autoscaling, mock_autoscaling_deprecated # noqa +from .awslambda import mock_lambda, mock_lambda_deprecated # noqa +from .cloudformation import mock_cloudformation, mock_cloudformation_deprecated # noqa +from .cloudwatch import mock_cloudwatch, mock_cloudwatch_deprecated # noqa +from .cognitoidentity import mock_cognitoidentity, mock_cognitoidentity_deprecated # noqa +from .cognitoidp import mock_cognitoidp, mock_cognitoidp_deprecated # noqa +from .config import mock_config # noqa +from .datapipeline import mock_datapipeline, mock_datapipeline_deprecated # noqa +from .dynamodb import mock_dynamodb, mock_dynamodb_deprecated # noqa +from .dynamodb2 import mock_dynamodb2, mock_dynamodb2_deprecated # noqa +from .dynamodbstreams import mock_dynamodbstreams # noqa +from .ec2 import mock_ec2, mock_ec2_deprecated # noqa +from .ecr import mock_ecr, mock_ecr_deprecated # noqa +from .ecs import mock_ecs, mock_ecs_deprecated # noqa +from .elb import mock_elb, mock_elb_deprecated # noqa +from .elbv2 import mock_elbv2 # noqa +from .emr import mock_emr, mock_emr_deprecated # noqa +from .events import mock_events # noqa +from .glacier import mock_glacier, mock_glacier_deprecated # noqa +from .glue import mock_glue # noqa +from .iam import mock_iam, mock_iam_deprecated # noqa +from .kinesis import mock_kinesis, mock_kinesis_deprecated # noqa +from .kms import mock_kms, mock_kms_deprecated # noqa +from .organizations import mock_organizations # noqa +from .opsworks import mock_opsworks, mock_opsworks_deprecated # noqa +from .polly import mock_polly # noqa +from .rds import mock_rds, mock_rds_deprecated # noqa +from .rds2 import mock_rds2, mock_rds2_deprecated # noqa +from .redshift import mock_redshift, mock_redshift_deprecated # noqa +from .resourcegroups import mock_resourcegroups # noqa +from .s3 import mock_s3, mock_s3_deprecated # noqa +from .ses import mock_ses, mock_ses_deprecated # noqa +from .secretsmanager import mock_secretsmanager # noqa +from .sns import mock_sns, mock_sns_deprecated # noqa +from .sqs import mock_sqs, mock_sqs_deprecated # noqa +from .stepfunctions import mock_stepfunctions # noqa +from .sts import mock_sts, mock_sts_deprecated # noqa +from .ssm import mock_ssm # noqa +from .route53 import mock_route53, mock_route53_deprecated # noqa +from .swf import mock_swf, mock_swf_deprecated # noqa +from .xray import mock_xray, mock_xray_client, XRaySegment # noqa +from .logs import mock_logs, mock_logs_deprecated # noqa +from .batch import mock_batch # noqa +from .resourcegroupstaggingapi import mock_resourcegroupstaggingapi # noqa +from .iot import mock_iot # noqa +from .iotdata import mock_iotdata # noqa try: diff --git a/moto/compat.py b/moto/compat.py index a92a5f67b..d7f5ab5e6 100644 --- a/moto/compat.py +++ b/moto/compat.py @@ -1,5 +1,5 @@ try: - from collections import OrderedDict # flake8: noqa + from collections import OrderedDict # noqa except ImportError: # python 2.6 or earlier, use backport - from ordereddict import OrderedDict # flake8: noqa + from ordereddict import OrderedDict # noqa diff --git a/moto/core/__init__.py b/moto/core/__init__.py index 801e675df..5e6a4472f 100644 --- a/moto/core/__init__.py +++ b/moto/core/__init__.py @@ -1,6 +1,6 @@ from __future__ import unicode_literals -from .models import BaseModel, BaseBackend, moto_api_backend # flake8: noqa +from .models import BaseModel, BaseBackend, moto_api_backend # noqa from .responses import ActionAuthenticatorMixin moto_api_backends = {"global": moto_api_backend} diff --git a/moto/dynamodb2/comparisons.py b/moto/dynamodb2/comparisons.py index c6aee7a68..734cfeeb3 100644 --- a/moto/dynamodb2/comparisons.py +++ b/moto/dynamodb2/comparisons.py @@ -91,12 +91,12 @@ class Op(object): # TODO add tests for all of these -EQ_FUNCTION = lambda item_value, test_value: item_value == test_value # flake8: noqa -NE_FUNCTION = lambda item_value, test_value: item_value != test_value # flake8: noqa -LE_FUNCTION = lambda item_value, test_value: item_value <= test_value # flake8: noqa -LT_FUNCTION = lambda item_value, test_value: item_value < test_value # flake8: noqa -GE_FUNCTION = lambda item_value, test_value: item_value >= test_value # flake8: noqa -GT_FUNCTION = lambda item_value, test_value: item_value > test_value # flake8: noqa +EQ_FUNCTION = lambda item_value, test_value: item_value == test_value # noqa +NE_FUNCTION = lambda item_value, test_value: item_value != test_value # noqa +LE_FUNCTION = lambda item_value, test_value: item_value <= test_value # noqa +LT_FUNCTION = lambda item_value, test_value: item_value < test_value # noqa +GE_FUNCTION = lambda item_value, test_value: item_value >= test_value # noqa +GT_FUNCTION = lambda item_value, test_value: item_value > test_value # noqa COMPARISON_FUNCS = { 'EQ': EQ_FUNCTION, diff --git a/moto/swf/models/__init__.py b/moto/swf/models/__init__.py index a8bc57f40..a2bc9a1f7 100644 --- a/moto/swf/models/__init__.py +++ b/moto/swf/models/__init__.py @@ -12,15 +12,15 @@ from ..exceptions import ( SWFTypeDeprecatedFault, SWFValidationException, ) -from .activity_task import ActivityTask # flake8: noqa -from .activity_type import ActivityType # flake8: noqa -from .decision_task import DecisionTask # flake8: noqa -from .domain import Domain # flake8: noqa -from .generic_type import GenericType # flake8: noqa -from .history_event import HistoryEvent # flake8: noqa -from .timeout import Timeout # flake8: noqa -from .workflow_type import WorkflowType # flake8: noqa -from .workflow_execution import WorkflowExecution # flake8: noqa +from .activity_task import ActivityTask # noqa +from .activity_type import ActivityType # noqa +from .decision_task import DecisionTask # noqa +from .domain import Domain # noqa +from .generic_type import GenericType # noqa +from .history_event import HistoryEvent # noqa +from .timeout import Timeout # noqa +from .workflow_type import WorkflowType # noqa +from .workflow_execution import WorkflowExecution # noqa from time import sleep KNOWN_SWF_TYPES = { diff --git a/scripts/scaffold.py b/scripts/scaffold.py index 6c83eeb50..be154f103 100755 --- a/scripts/scaffold.py +++ b/scripts/scaffold.py @@ -119,7 +119,7 @@ def append_mock_to_init_py(service): filtered_lines = [_ for _ in lines if re.match('^from.*mock.*$', _)] last_import_line_index = lines.index(filtered_lines[-1]) - new_line = 'from .{} import mock_{} # flake8: noqa'.format(get_escaped_service(service), get_escaped_service(service)) + new_line = 'from .{} import mock_{} # noqa'.format(get_escaped_service(service), get_escaped_service(service)) lines.insert(last_import_line_index + 1, new_line) body = '\n'.join(lines) + '\n' diff --git a/tests/test_ec2/test_vpcs.py b/tests/test_ec2/test_vpcs.py index ad17deb3c..903c0630d 100644 --- a/tests/test_ec2/test_vpcs.py +++ b/tests/test_ec2/test_vpcs.py @@ -1,6 +1,6 @@ from __future__ import unicode_literals # Ensure 'assert_raises' context manager support for Python 2.6 -import tests.backport_assert_raises # flake8: noqa +import tests.backport_assert_raises # noqa from nose.tools import assert_raises from moto.ec2.exceptions import EC2ClientError from botocore.exceptions import ClientError From 84fb52d0a26603caa8522c7ad90b90ea7ed3d6c5 Mon Sep 17 00:00:00 2001 From: Chih-Hsuan Yen Date: Sun, 27 Oct 2019 21:00:01 +0800 Subject: [PATCH 10/15] Fix remaining flake8 issues Disabling W504 and W605 for now as there are too many instances. --- moto/__init__.py | 2 +- moto/cloudwatch/responses.py | 4 ++-- moto/dynamodb2/comparisons.py | 23 ++++++++--------------- moto/ec2/models.py | 14 +++++++------- moto/ecs/models.py | 4 ++-- moto/iotdata/models.py | 2 +- moto/rds2/models.py | 4 ++-- moto/resourcegroupstaggingapi/models.py | 2 +- moto/ssm/models.py | 6 +++--- tox.ini | 2 +- 10 files changed, 28 insertions(+), 35 deletions(-) diff --git a/moto/__init__.py b/moto/__init__.py index 36d294657..592a20b07 100644 --- a/moto/__init__.py +++ b/moto/__init__.py @@ -1,5 +1,5 @@ from __future__ import unicode_literals -import logging +# import logging # logging.getLogger('boto').setLevel(logging.CRITICAL) __title__ = 'moto' diff --git a/moto/cloudwatch/responses.py b/moto/cloudwatch/responses.py index bf176e1be..b9f20f559 100644 --- a/moto/cloudwatch/responses.py +++ b/moto/cloudwatch/responses.py @@ -96,11 +96,11 @@ class CloudWatchResponse(BaseResponse): extended_statistics = self._get_param('ExtendedStatistics') dimensions = self._get_param('Dimensions') if unit or extended_statistics or dimensions: - raise NotImplemented() + raise NotImplementedError() # TODO: this should instead throw InvalidParameterCombination if not statistics: - raise NotImplemented("Must specify either Statistics or ExtendedStatistics") + raise NotImplementedError("Must specify either Statistics or ExtendedStatistics") datapoints = self.cloudwatch_backend.get_metric_statistics(namespace, metric_name, start_time, end_time, period, statistics) template = self.response_template(GET_METRIC_STATISTICS_TEMPLATE) diff --git a/moto/dynamodb2/comparisons.py b/moto/dynamodb2/comparisons.py index 734cfeeb3..ab0eeb85d 100644 --- a/moto/dynamodb2/comparisons.py +++ b/moto/dynamodb2/comparisons.py @@ -1,7 +1,5 @@ from __future__ import unicode_literals import re -import six -import re from collections import deque from collections import namedtuple @@ -48,11 +46,11 @@ def get_expected(expected): path = AttributePath([key]) if 'Exists' in cond: if cond['Exists']: - conditions.append(FuncAttrExists(path)) + conditions.append(FuncAttrExists(path)) else: - conditions.append(FuncAttrNotExists(path)) + conditions.append(FuncAttrNotExists(path)) elif 'Value' in cond: - conditions.append(OpEqual(path, AttributeValue(cond['Value']))) + conditions.append(OpEqual(path, AttributeValue(cond['Value']))) elif 'ComparisonOperator' in cond: operator_name = cond['ComparisonOperator'] values = [ @@ -221,7 +219,6 @@ class ConditionExpressionParser: # -------------- LITERAL = 'LITERAL' - class Nonterminal: """Enum defining nonterminals for productions.""" @@ -240,7 +237,6 @@ class ConditionExpressionParser: RIGHT_PAREN = 'RIGHT_PAREN' WHITESPACE = 'WHITESPACE' - Node = namedtuple('Node', ['nonterminal', 'kind', 'text', 'value', 'children']) def _lex_condition_expression(self): @@ -286,11 +282,10 @@ class ConditionExpressionParser: if match: match_text = match.group() break - else: # pragma: no cover + else: # pragma: no cover raise ValueError("Cannot parse condition starting at: " + - remaining_expression) + remaining_expression) - value = match_text node = self.Node( nonterminal=nonterminal, kind=self.Kind.LITERAL, @@ -351,7 +346,6 @@ class ConditionExpressionParser: 'size', } - if name.lower() in reserved: # e.g. AND nonterminal = reserved[name.lower()] @@ -748,14 +742,13 @@ class ConditionExpressionParser: elif node.kind == self.Kind.FUNCTION: # size() function_node = node.children[0] - arguments = node.children[1:] + arguments = node.children[1:] function_name = function_node.value arguments = [self._make_operand(arg) for arg in arguments] return FUNC_CLASS[function_name](*arguments) - else: # pragma: no cover + else: # pragma: no cover raise ValueError("Unknown operand: %r" % node) - def _make_op_condition(self, node): if node.kind == self.Kind.OR: lhs, rhs = node.children @@ -796,7 +789,7 @@ class ConditionExpressionParser: return COMPARATOR_CLASS[comparator.value]( self._make_operand(lhs), self._make_operand(rhs)) - else: # pragma: no cover + else: # pragma: no cover raise ValueError("Unknown expression node kind %r" % node.kind) def _assert(self, condition, message, nodes): diff --git a/moto/ec2/models.py b/moto/ec2/models.py index 10d6f2b28..69fd844fd 100644 --- a/moto/ec2/models.py +++ b/moto/ec2/models.py @@ -1229,7 +1229,7 @@ class AmiBackend(object): images = [ami for ami in images if ami.id in ami_ids] if len(images) == 0: - raise InvalidAMIIdError(ami_ids) + raise InvalidAMIIdError(ami_ids) else: # Limit images by launch permissions if exec_users: @@ -3720,8 +3720,8 @@ class VPNConnection(TaggedEC2Resource): self.static_routes = None def get_filter_value(self, filter_name): - return super(VPNConnection, self).get_filter_value( - filter_name, 'DescribeVpnConnections') + return super(VPNConnection, self).get_filter_value( + filter_name, 'DescribeVpnConnections') class VPNConnectionBackend(object): @@ -3950,8 +3950,8 @@ class VpnGateway(TaggedEC2Resource): super(VpnGateway, self).__init__() def get_filter_value(self, filter_name): - return super(VpnGateway, self).get_filter_value( - filter_name, 'DescribeVpnGateways') + return super(VpnGateway, self).get_filter_value( + filter_name, 'DescribeVpnGateways') class VpnGatewayAttachment(object): @@ -4015,8 +4015,8 @@ class CustomerGateway(TaggedEC2Resource): super(CustomerGateway, self).__init__() def get_filter_value(self, filter_name): - return super(CustomerGateway, self).get_filter_value( - filter_name, 'DescribeCustomerGateways') + return super(CustomerGateway, self).get_filter_value( + filter_name, 'DescribeCustomerGateways') class CustomerGatewayBackend(object): diff --git a/moto/ecs/models.py b/moto/ecs/models.py index 5aa9ae2cb..5ba420508 100644 --- a/moto/ecs/models.py +++ b/moto/ecs/models.py @@ -158,8 +158,8 @@ class TaskDefinition(BaseObject): if (original_resource.family != family or original_resource.container_definitions != container_definitions or original_resource.volumes != volumes): - # currently TaskRoleArn isn't stored at TaskDefinition - # instances + # currently TaskRoleArn isn't stored at TaskDefinition + # instances ecs_backend = ecs_backends[region_name] ecs_backend.deregister_task_definition(original_resource.arn) return ecs_backend.register_task_definition( diff --git a/moto/iotdata/models.py b/moto/iotdata/models.py index fec066f07..010017fc3 100644 --- a/moto/iotdata/models.py +++ b/moto/iotdata/models.py @@ -163,7 +163,7 @@ class IoTDataPlaneBackend(BaseBackend): raise InvalidRequestException('State contains an invalid node') if 'version' in payload and thing.thing_shadow.version != payload['version']: - raise ConflictException('Version conflict') + raise ConflictException('Version conflict') new_shadow = FakeShadow.create_from_previous_version(thing.thing_shadow, payload) thing.thing_shadow = new_shadow return thing.thing_shadow diff --git a/moto/rds2/models.py b/moto/rds2/models.py index cd56599e6..b9a250646 100644 --- a/moto/rds2/models.py +++ b/moto/rds2/models.py @@ -875,8 +875,8 @@ class RDS2Backend(BaseBackend): database = self.describe_databases(db_instance_identifier)[0] # todo: certain rds types not allowed to be stopped at this time. if database.is_replica or database.multi_az: - # todo: more db types not supported by stop/start instance api - raise InvalidDBClusterStateFaultError(db_instance_identifier) + # todo: more db types not supported by stop/start instance api + raise InvalidDBClusterStateFaultError(db_instance_identifier) if database.status != 'available': raise InvalidDBInstanceStateError(db_instance_identifier, 'stop') if db_snapshot_identifier: diff --git a/moto/resourcegroupstaggingapi/models.py b/moto/resourcegroupstaggingapi/models.py index 3f15017cc..68430a5bf 100644 --- a/moto/resourcegroupstaggingapi/models.py +++ b/moto/resourcegroupstaggingapi/models.py @@ -244,7 +244,7 @@ class ResourceGroupsTaggingAPIBackend(BaseBackend): def get_kms_tags(kms_key_id): result = [] for tag in self.kms_backend.list_resource_tags(kms_key_id): - result.append({'Key': tag['TagKey'], 'Value': tag['TagValue']}) + result.append({'Key': tag['TagKey'], 'Value': tag['TagValue']}) return result if not resource_type_filters or 'kms' in resource_type_filters: diff --git a/moto/ssm/models.py b/moto/ssm/models.py index 39bd63ede..e5e52c0c9 100644 --- a/moto/ssm/models.py +++ b/moto/ssm/models.py @@ -210,9 +210,9 @@ class Command(BaseModel): 'An error occurred (InvocationDoesNotExist) when calling the GetCommandInvocation operation') if plugin_name is not None and invocation['PluginName'] != plugin_name: - raise RESTError( - 'InvocationDoesNotExist', - 'An error occurred (InvocationDoesNotExist) when calling the GetCommandInvocation operation') + raise RESTError( + 'InvocationDoesNotExist', + 'An error occurred (InvocationDoesNotExist) when calling the GetCommandInvocation operation') return invocation diff --git a/tox.ini b/tox.ini index 570b5790f..1235a8e2b 100644 --- a/tox.ini +++ b/tox.ini @@ -15,5 +15,5 @@ commands = nosetests {posargs} [flake8] -ignore = E128,E501 +ignore = E128,E501,W504,W605 exclude = moto/packages,dist From 8313142f449887133638d36806b0d21e85644e9b Mon Sep 17 00:00:00 2001 From: Chih-Hsuan Yen Date: Sun, 27 Oct 2019 23:26:49 +0800 Subject: [PATCH 11/15] Use Buster container images for Python 3.8 --- .travis.yml | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 3f73af5aa..c69c3ea1f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -18,7 +18,14 @@ install: python setup.py sdist if [ "$TEST_SERVER_MODE" = "true" ]; then - docker run --rm -t --name motoserver -e TEST_SERVER_MODE=true -e AWS_SECRET_ACCESS_KEY=server_secret -e AWS_ACCESS_KEY_ID=server_key -v `pwd`:/moto -p 5000:5000 -v /var/run/docker.sock:/var/run/docker.sock python:${TRAVIS_PYTHON_VERSION}-stretch /moto/travis_moto_server.sh & + if [ "$TRAVIS_PYTHON_VERSION" = "3.8" ]; then + # Python 3.8 does not provide Stretch images yet [1] + # [1] https://github.com/docker-library/python/issues/428 + PYTHON_DOCKER_TAG=${TRAVIS_PYTHON_VERSION}-buster + else + PYTHON_DOCKER_TAG=${TRAVIS_PYTHON_VERSION}-stretch + fi + docker run --rm -t --name motoserver -e TEST_SERVER_MODE=true -e AWS_SECRET_ACCESS_KEY=server_secret -e AWS_ACCESS_KEY_ID=server_key -v `pwd`:/moto -p 5000:5000 -v /var/run/docker.sock:/var/run/docker.sock python:${PYTHON_DOCKER_TAG} /moto/travis_moto_server.sh & fi travis_retry pip install boto==2.45.0 travis_retry pip install boto3 From c5059ad3d156e3d0b16034867e759d3a91620ce2 Mon Sep 17 00:00:00 2001 From: gruebel Date: Mon, 28 Oct 2019 17:21:01 +0100 Subject: [PATCH 12/15] Fix test server error --- tests/test_sqs/test_sqs.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/test_sqs/test_sqs.py b/tests/test_sqs/test_sqs.py index 2db457f72..a2111c9d7 100644 --- a/tests/test_sqs/test_sqs.py +++ b/tests/test_sqs/test_sqs.py @@ -164,9 +164,7 @@ def test_get_queue_url(): response = client.get_queue_url(QueueName='test-queue') - response['QueueUrl'].should.equal( - 'https://queue.amazonaws.com/123456789012/test-queue' - ) + response.should.have.key('QueueUrl').which.should.contain('test-queue') @mock_sqs From 503bc333ca44e32ce0d1ee3afbb1f3a9ab03364c Mon Sep 17 00:00:00 2001 From: Mike Grima Date: Tue, 29 Oct 2019 14:35:13 -0700 Subject: [PATCH 13/15] Small fix for S3-AWS Config compatibility - Small bug in tags with AWS Config - Aggregated results lack "tags" in the result set - Buckets also add a supplementary configuration of "BucketTaggingConfiguration" --- moto/config/models.py | 14 +++++++++----- moto/s3/models.py | 4 ++++ tests/test_config/test_config.py | 8 ++++++++ tests/test_s3/test_s3.py | 7 ++++++- 4 files changed, 27 insertions(+), 6 deletions(-) diff --git a/moto/config/models.py b/moto/config/models.py index 1ea9ba043..3b3544f2d 100644 --- a/moto/config/models.py +++ b/moto/config/models.py @@ -744,8 +744,9 @@ class ConfigBackend(BaseBackend): def list_aggregate_discovered_resources(self, aggregator_name, resource_type, filters, limit, next_token): """This will query against the mocked AWS Config listing function that must exist for the resource backend. - As far a moto goes -- the only real difference between this function and the `list_discovered_resources` function is that - this will require a Config Aggregator be set up a priori and can search based on resource regions. + + As far a moto goes -- the only real difference between this function and the `list_discovered_resources` function is that + this will require a Config Aggregator be set up a priori and can search based on resource regions. :param aggregator_name: :param resource_type: @@ -796,9 +797,9 @@ class ConfigBackend(BaseBackend): def get_resource_config_history(self, resource_type, id, backend_region): """Returns the configuration of an item in the AWS Config format of the resource for the current regional backend. - NOTE: This is --NOT-- returning history as it is not supported in moto at this time. (PR's welcome!) - As such, the later_time, earlier_time, limit, and next_token are ignored as this will only - return 1 item. (If no items, it raises an exception) + NOTE: This is --NOT-- returning history as it is not supported in moto at this time. (PR's welcome!) + As such, the later_time, earlier_time, limit, and next_token are ignored as this will only + return 1 item. (If no items, it raises an exception) """ # If the type isn't implemented then we won't find the item: if resource_type not in RESOURCE_MAP: @@ -897,6 +898,9 @@ class ConfigBackend(BaseBackend): item['accountId'] = DEFAULT_ACCOUNT_ID + # The 'tags' field is not included in aggregate results for some reason... + item.pop('tags', None) + found.append(item) return {'BaseConfigurationItems': found, 'UnprocessedResourceIdentifiers': not_found} diff --git a/moto/s3/models.py b/moto/s3/models.py index 9314d2d05..8c4a058ee 100644 --- a/moto/s3/models.py +++ b/moto/s3/models.py @@ -934,6 +934,10 @@ class FakeBucket(BaseModel): # This is a dobule-wrapped JSON for some reason... s_config = {'AccessControlList': json.dumps(json.dumps(self.acl.to_config_dict()))} + # Tagging is special: + if config_dict['tags']: + s_config['BucketTaggingConfiguration'] = json.dumps({'tagSets': [{'tags': config_dict['tags']}]}) + # TODO implement Accelerate Configuration: s_config['BucketAccelerateConfiguration'] = {'status': None} diff --git a/tests/test_config/test_config.py b/tests/test_config/test_config.py index 824e0a5c2..8b9a5d877 100644 --- a/tests/test_config/test_config.py +++ b/tests/test_config/test_config.py @@ -1,3 +1,4 @@ +import json from datetime import datetime, timedelta import boto3 @@ -1314,10 +1315,12 @@ def test_batch_get_aggregate_resource_config(): s3_client = boto3.client('s3', region_name='us-west-2') for x in range(0, 10): s3_client.create_bucket(Bucket='bucket{}'.format(x), CreateBucketConfiguration={'LocationConstraint': 'us-west-2'}) + s3_client.put_bucket_tagging(Bucket='bucket{}'.format(x), Tagging={'TagSet': [{'Key': 'Some', 'Value': 'Tag'}]}) s3_client_eu = boto3.client('s3', region_name='eu-west-1') for x in range(10, 12): s3_client_eu.create_bucket(Bucket='eu-bucket{}'.format(x), CreateBucketConfiguration={'LocationConstraint': 'eu-west-1'}) + s3_client.put_bucket_tagging(Bucket='eu-bucket{}'.format(x), Tagging={'TagSet': [{'Key': 'Some', 'Value': 'Tag'}]}) # Now try with resources that exist and ones that don't: identifiers = [{'SourceAccountId': DEFAULT_ACCOUNT_ID, 'SourceRegion': 'us-west-2', 'ResourceType': 'AWS::S3::Bucket', @@ -1339,6 +1342,11 @@ def test_batch_get_aggregate_resource_config(): assert not missing_buckets + # Verify that 'tags' is not in the result set: + for b in result['BaseConfigurationItems']: + assert not b.get('tags') + assert json.loads(b['supplementaryConfiguration']['BucketTaggingConfiguration']) == {'tagSets': [{'tags': {'Some': 'Tag'}}]} + # Verify that if the resource name and ID are correct that things are good: identifiers = [{'SourceAccountId': DEFAULT_ACCOUNT_ID, 'SourceRegion': 'us-west-2', 'ResourceType': 'AWS::S3::Bucket', 'ResourceId': 'bucket1', 'ResourceName': 'bucket1'}] diff --git a/tests/test_s3/test_s3.py b/tests/test_s3/test_s3.py index e596a3c64..6de511ef7 100644 --- a/tests/test_s3/test_s3.py +++ b/tests/test_s3/test_s3.py @@ -3718,6 +3718,8 @@ def test_s3_config_dict(): assert bucket1_result['awsRegion'] == 'us-west-2' assert bucket1_result['resourceName'] == bucket1_result['resourceId'] == 'bucket1' assert bucket1_result['tags'] == {'someTag': 'someValue', 'someOtherTag': 'someOtherValue'} + assert json.loads(bucket1_result['supplementaryConfiguration']['BucketTaggingConfiguration']) == \ + {'tagSets': [{'tags': bucket1_result['tags']}]} assert isinstance(bucket1_result['configuration'], str) exist_list = ['AccessControlList', 'BucketAccelerateConfiguration', 'BucketLoggingConfiguration', 'BucketPolicy', 'IsRequesterPaysEnabled', 'BucketNotificationConfiguration'] @@ -3748,5 +3750,8 @@ def test_s3_config_dict(): assert not s3_config_query.get_config_resource('bucket1', resource_name='eu-bucket-1') # Verify that no bucket policy returns the proper value: - assert json.loads(s3_config_query.get_config_resource('logbucket')['supplementaryConfiguration']['BucketPolicy']) == \ + logging_bucket = s3_config_query.get_config_resource('logbucket') + assert json.loads(logging_bucket['supplementaryConfiguration']['BucketPolicy']) == \ {'policyText': None} + assert not logging_bucket['tags'] + assert not logging_bucket['supplementaryConfiguration'].get('BucketTaggingConfiguration') From 86e53c3db346e23f1a997ff4f51286bfd194b1f7 Mon Sep 17 00:00:00 2001 From: gruebel Date: Wed, 30 Oct 2019 17:37:38 +0100 Subject: [PATCH 14/15] Remove dead code --- moto/sqs/responses.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/moto/sqs/responses.py b/moto/sqs/responses.py index e6982b795..ad46df723 100644 --- a/moto/sqs/responses.py +++ b/moto/sqs/responses.py @@ -251,8 +251,6 @@ class SQSResponse(BaseResponse): message_attributes = parse_message_attributes( self.querystring, base='SendMessageBatchRequestEntry.{}.'.format(index)) - if type(message_attributes) == tuple: - return message_attributes[0], message_attributes[1] entries[index] = { 'Id': value[0], From 5cccb03c9143a289d83552858dcbfaeacbd80673 Mon Sep 17 00:00:00 2001 From: Patrick Mende Date: Wed, 30 Oct 2019 14:08:24 -0700 Subject: [PATCH 15/15] Remove newlines from XML responses This is a second attempt at resolving the issues with producing an XML consistent with what is produced from AWS (i.e., no spaces/new lines between tags). Another attempt (https://github.com/spulec/moto/pull/2205) is currently failing in tests. This attempt uses precompiled regex patterns as class attributes of the `_TemplateEnvironmentMixin` to remove trailing spaces and newlines after a `">"`, and preceding newlines/spaces before a `"<"`. This *explicitly* wasn't done with a single regex to ensure that even things like `"...\n 12345\n "` are properly collapsed. --- moto/core/responses.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/moto/core/responses.py b/moto/core/responses.py index 213fa278c..6d7197674 100644 --- a/moto/core/responses.py +++ b/moto/core/responses.py @@ -83,6 +83,8 @@ class DynamicDictLoader(DictLoader): class _TemplateEnvironmentMixin(object): + LEFT_PATTERN = re.compile(r"[\s\n]+<") + RIGHT_PATTERN = re.compile(r">[\s\n]+") def __init__(self): super(_TemplateEnvironmentMixin, self).__init__() @@ -101,7 +103,12 @@ class _TemplateEnvironmentMixin(object): def response_template(self, source): template_id = id(source) if not self.contains_template(template_id): - self.loader.update({template_id: source}) + collapsed = re.sub( + self.RIGHT_PATTERN, + ">", + re.sub(self.LEFT_PATTERN, "<", source) + ) + self.loader.update({template_id: collapsed}) self.environment = Environment(loader=self.loader, autoescape=self.should_autoescape, trim_blocks=True, lstrip_blocks=True) return self.environment.get_template(template_id)