Mock AWS_DEFAULT_REGION for all unit tests #1710

nadove-ucsc · 2020-04-16T02:35:27Z

No description provided.

codecov · 2020-04-16T02:52:07Z

Codecov Report

Merging #1710 into develop will decrease coverage by 0.02%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop    #1710      +/-   ##
===========================================
- Coverage    83.62%   83.60%   -0.03%     
===========================================
  Files          104      104              
  Lines         9951     9961      +10     
===========================================
+ Hits          8322     8328       +6     
- Misses        1629     1633       +4

Impacted Files	Coverage Δ
test/azul_test_case.py	`46.01% <100.00%> (+3.56%)`	⬆️
test/dynamo_test_case.py	`100.00% <100.00%> (ø)`
test/health_check_test_case.py	`98.44% <100.00%> (-0.02%)`	⬇️
test/indexer/test_hca_indexer.py	`99.32% <100.00%> (-0.01%)`	⬇️
test/version_table_test_case.py	`93.75% <100.00%> (+0.41%)`	⬆️
src/azul/queues.py	`0.00% <0.00%> (ø)`
test/integration_test.py	`0.00% <0.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b177977...6722202. Read the comment docs.

coveralls · 2020-04-16T02:52:10Z

Pull Request Test Coverage Report for Build 8098

16 of 16 (100.0%) changed or added relevant lines in 5 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.009%) to 82.991%

Totals
Change from base Build 8083:	0.009%
Covered Lines:	9080
Relevant Lines:	10941

💛 - Coveralls

jessebrennan

This looks good to me 👍

hannes-ucsc · 2020-04-16T23:33:18Z

test/azul_test_case.py

@@ -52,6 +56,10 @@ class AzulTestCase(TestCase):
    get_credentials_boto3 = None
    _saved_boto3_default_session = None

+    # Region corresponds to south asia (Mumbai). Must be a real region because
+    # SQS will verify this.


Moto checks this, not SQS.

True, but moto defers to boto when retrieving the list of valid regions.

hannes-ucsc · 2020-04-17T00:09:08Z

test/azul_test_case.py

@@ -52,6 +56,10 @@ class AzulTestCase(TestCase):
    get_credentials_boto3 = None
    _saved_boto3_default_session = None

+    # Region corresponds to south asia (Mumbai). Must be a real region because
+    # SQS will verify this.
+    _aws_region_mock = patch.dict(os.environ, AWS_DEFAULT_REGION='ap-south-1')


Suggested change

_aws_region_mock = patch.dict(os.environ, AWS_DEFAULT_REGION='ap-south-1')

_aws_region_mock = patch.dict(os.environ, AWS_DEFAULT_REGION='us-gov-west-1')

we are pretty much guaranteed to not have access to that region.

Moto/boto does not consider this to be a valid region in certain contexts:

====================================================================== ERROR: test_versioning (service.test_version_service.TestVersionService) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/travis/build/DataBiosphere/azul/test/service/test_version_service.py", line 19, in setUp super().setUp() File "/home/travis/virtualenv/python3.6.7/lib/python3.6/site-packages/moto/core/models.py", line 80, in wrapper result = func(*args, **kwargs) File "/home/travis/build/DataBiosphere/azul/test/version_table_test_case.py", line 23, in setUp dict(AttributeName=VersionService.key_name, KeyType='HASH'), File "/home/travis/virtualenv/python3.6.7/lib/python3.6/site-packages/botocore/client.py", line 357, in _api_call return self._make_api_call(operation_name, kwargs) File "/home/travis/virtualenv/python3.6.7/lib/python3.6/site-packages/botocore/client.py", line 648, in _make_api_call operation_model, request_dict, request_context) File "/home/travis/virtualenv/python3.6.7/lib/python3.6/site-packages/botocore/client.py", line 667, in _make_request return self._endpoint.make_request(operation_model, request_dict) File "/home/travis/virtualenv/python3.6.7/lib/python3.6/site-packages/botocore/endpoint.py", line 102, in make_request return self._send_request(request_dict, operation_model) File "/home/travis/virtualenv/python3.6.7/lib/python3.6/site-packages/botocore/endpoint.py", line 137, in _send_request success_response, exception): File "/home/travis/virtualenv/python3.6.7/lib/python3.6/site-packages/botocore/endpoint.py", line 231, in _needs_retry caught_exception=caught_exception, request_dict=request_dict) File "/home/travis/virtualenv/python3.6.7/lib/python3.6/site-packages/botocore/hooks.py", line 356, in emit return self._emitter.emit(aliased_event_name, **kwargs) File "/home/travis/virtualenv/python3.6.7/lib/python3.6/site-packages/botocore/hooks.py", line 228, in emit return self._emit(event_name, kwargs) File "/home/travis/virtualenv/python3.6.7/lib/python3.6/site-packages/botocore/hooks.py", line 211, in _emit response = handler(**kwargs) File "/home/travis/virtualenv/python3.6.7/lib/python3.6/site-packages/botocore/retryhandler.py", line 183, in __call__ if self._checker(attempts, response, caught_exception): File "/home/travis/virtualenv/python3.6.7/lib/python3.6/site-packages/botocore/retryhandler.py", line 251, in __call__ caught_exception) File "/home/travis/virtualenv/python3.6.7/lib/python3.6/site-packages/botocore/retryhandler.py", line 269, in _should_retry return self._checker(attempt_number, response, caught_exception) File "/home/travis/virtualenv/python3.6.7/lib/python3.6/site-packages/botocore/retryhandler.py", line 317, in __call__ caught_exception) File "/home/travis/virtualenv/python3.6.7/lib/python3.6/site-packages/botocore/retryhandler.py", line 223, in __call__ attempt_number, caught_exception) File "/home/travis/virtualenv/python3.6.7/lib/python3.6/site-packages/botocore/retryhandler.py", line 359, in _check_caught_exception raise caught_exception File "/home/travis/virtualenv/python3.6.7/lib/python3.6/site-packages/botocore/endpoint.py", line 197, in _do_get_response responses = self._event_emitter.emit(event_name, request=request) File "/home/travis/virtualenv/python3.6.7/lib/python3.6/site-packages/botocore/hooks.py", line 356, in emit return self._emitter.emit(aliased_event_name, **kwargs) File "/home/travis/virtualenv/python3.6.7/lib/python3.6/site-packages/botocore/hooks.py", line 228, in emit return self._emit(event_name, kwargs) File "/home/travis/virtualenv/python3.6.7/lib/python3.6/site-packages/botocore/hooks.py", line 211, in _emit response = handler(**kwargs) File "/home/travis/virtualenv/python3.6.7/lib/python3.6/site-packages/moto/core/models.py", line 298, in __call__ status, headers, body = response_callback(request, request.url, request.headers) File "/home/travis/virtualenv/python3.6.7/lib/python3.6/site-packages/moto/core/responses.py", line 117, in dispatch return cls()._dispatch(*args, **kwargs) File "/home/travis/virtualenv/python3.6.7/lib/python3.6/site-packages/moto/core/responses.py", line 207, in _dispatch return self.call_action() File "/home/travis/virtualenv/python3.6.7/lib/python3.6/site-packages/moto/core/utils.py", line 264, in _wrapper response = f(*args, **kwargs) File "/home/travis/virtualenv/python3.6.7/lib/python3.6/site-packages/moto/dynamodb2/responses.py", line 65, in call_action response = getattr(self, endpoint)() File "/home/travis/virtualenv/python3.6.7/lib/python3.6/site-packages/moto/dynamodb2/responses.py", line 119, in create_table table = self.dynamodb_backend.create_table(table_name, File "/home/travis/virtualenv/python3.6.7/lib/python3.6/site-packages/moto/dynamodb2/responses.py", line 57, in dynamodb_backend return dynamodb_backends[self.region] KeyError: 'us-gov-west-1' ---------------------------------------------------------------------- Ran 322 tests in 817.926s

I believe the list of regions that will not trigger this error can be found in .venv/lib/python3.6/site-packages/botocore/data/endpoints.json

$ cat .venv/lib/python3.6/site-packages/botocore/data/endpoints.json | jq .partitions[].regions | jq keys[] "ap-east-1" "ap-northeast-1" "ap-northeast-2" "ap-south-1" "ap-southeast-1" "ap-southeast-2" "ca-central-1" "eu-central-1" "eu-north-1" "eu-west-1" "eu-west-2" "eu-west-3" "me-south-1" "sa-east-1" "us-east-1" "us-east-2" "us-west-1" "us-west-2" "cn-north-1" "cn-northwest-1" "us-gov-east-1" "us-gov-west-1" "us-iso-east-1" "us-isob-east-1"

More directly and correctly:

>>> import moto.dynamodb2.models as m >>> m.dynamodb_backends.keys() dict_keys(['ap-east-1', 'ap-northeast-1', 'ap-northeast-2', 'ap-south-1', 'ap-southeast-1', 'ap-southeast-2', 'ca-central-1', 'eu-central-1', 'eu-north-1', 'eu-west-1', 'eu-west-2', 'eu-west-3', 'me-south-1', 'sa-east-1', 'us-east-1', 'us-east-2', 'us-west-1', 'us-west-2'])

test/azul_test_case.py

hannes-ucsc · 2020-04-17T17:21:38Z

test/azul_test_case.py

@@ -82,8 +87,15 @@ def dummy_get_credentials(self):
        botocore.session.Session.get_credentials = dummy_get_credentials
        boto3.session.Session.get_credentials = dummy_get_credentials

+        # Ensure that mock leakages fail by targeting another region
+        # Moto will verify that region is valid.


Suggested change

# Moto will verify that region is valid.

# We can't use an artificial region name since Moto validates it.

hannes-ucsc · 2020-04-17T17:22:05Z

test/azul_test_case.py

+        # Ensure that mock leakages fail by targeting another region
+        # Moto will verify that region is valid.
+        # We are pretty much guaranteed not to have access to this region.
+        cls._aws_region_mock = patch.dict(os.environ, AWS_DEFAULT_REGION='us-gov-west-1')


Any other gov region we could use?

I don't believe so: #1710 (comment)

If it's that important I could try to mock moto's internals so that a made-up region is mapped to a valid backend.

Actually that almost certainly won't work. Moto retrieves the list of available regions from boto at import time so it's too late for setUpClass to patch anything. Furthermore the separate moto services (e.g. dynamodb, sqs) have their own regions lists, so to overwrite boto's list we'd need to patch each each individually, which seems very brittle and tedious. My current belief is that our best option is to select one the regions listed above.

test/azul_test_case.py

hannes-ucsc · 2020-04-24T06:54:35Z

test/azul_test_case.py

+        # Ensure that mock leakages fail by targeting another region.
+        # Moto verifies the region against a list of supported backends, so we
+        # can't use an artifical one (see site-packages/boto/endpoints.json).
+        cls._aws_region_mock = patch.dict(os.environ, AWS_DEFAULT_REGION='ap-south-1')


Unfortunately, because this is an real region in the default AWS partition, this doesn't actually prevent access to real resources. I understand the difficulty in patching all of the region info code in moto, boto and boto3 and don't have a good solution either but I don't see how this buys us any additional security.

hannes-ucsc · 2020-04-24T06:56:57Z

Moved to icebox. Considering closing it.

hannes-ucsc

IIRC, the only test that failed when this was set to "us-gov-west-1", were the ones that used the DynamoDB mocks. Could you verify that?

nadove-ucsc · 2020-04-30T02:07:11Z

@hannes-ucsc You are correct. There are only two classes that fail when the region mock is set to us-gov-west-1, and in both cases the error occurs within the dynamodb backed. They are TestPortalService and TestVersionService. If these classes are skip-ed then all unit tests pass on the branch.

hannes-ucsc · 2020-04-30T19:14:54Z

In that case, we should use a region from the gov partition, probably us-gov-west-1 by default and ap-south-1 for those two test cases. AzulTestCase could expose a class attribute that these two concrete test cases override.

hannes-ucsc · 2020-05-01T01:17:56Z

test/azul_test_case.py

        boto3.session.Session.get_credentials = cls.get_credentials_boto3
        botocore.session.Session.get_credentials = cls.get_credentials_botocore
        boto3.DEFAULT_SESSION = cls._saved_boto3_default_session
        os.environ['AZUL_AWS_ACCOUNT_ID'] = cls.aws_account_id
        super().tearDownClass()

+    @classmethod
+    def get_aws_mock_region(cls):


Can you try a simple class attribute, not a class method?

github-actions bot added the orange [process] Done by the Azul team label Apr 16, 2020

nadove-ucsc requested a review from jessebrennan April 16, 2020 02:53

nadove-ucsc assigned jessebrennan Apr 16, 2020

jessebrennan approved these changes Apr 16, 2020

View reviewed changes

jessebrennan removed their assignment Apr 16, 2020

nadove-ucsc assigned hannes-ucsc Apr 16, 2020

nadove-ucsc requested a review from hannes-ucsc April 16, 2020 18:38

hannes-ucsc reviewed Apr 16, 2020

View reviewed changes

hannes-ucsc requested changes Apr 17, 2020

View reviewed changes

hannes-ucsc removed their assignment Apr 17, 2020

hannes-ucsc added the 1 review [process] Lead requested changes once label Apr 17, 2020

nadove-ucsc force-pushed the issues/noah-aviel-dove/1498-unit-tests-special-region branch from 78e893a to 0dd96b2 Compare April 17, 2020 08:27

hannes-ucsc requested changes Apr 17, 2020

View reviewed changes

nadove-ucsc force-pushed the issues/noah-aviel-dove/1498-unit-tests-special-region branch from 0dd96b2 to 9b394b3 Compare April 17, 2020 21:55

nadove-ucsc requested a review from hannes-ucsc April 17, 2020 23:55

nadove-ucsc assigned hannes-ucsc Apr 17, 2020

hannes-ucsc reviewed Apr 24, 2020

View reviewed changes

hannes-ucsc removed their assignment Apr 24, 2020

hannes-ucsc requested changes Apr 27, 2020

View reviewed changes

melainalegaspi assigned hannes-ucsc Apr 30, 2020

hannes-ucsc removed their assignment Apr 30, 2020

nadove-ucsc force-pushed the issues/noah-aviel-dove/1498-unit-tests-special-region branch from 9b394b3 to 6de519f Compare April 30, 2020 22:14

nadove-ucsc requested a review from hannes-ucsc April 30, 2020 23:03

nadove-ucsc assigned hannes-ucsc Apr 30, 2020

hannes-ucsc requested changes May 1, 2020

View reviewed changes

hannes-ucsc removed their assignment May 1, 2020

hannes-ucsc added 2 reviews [process] Lead requested changes twice and removed 1 review [process] Lead requested changes once labels May 1, 2020

hannes-ucsc mentioned this pull request May 1, 2020

Implemented /health/failures endpoint (#1260) #1305

Closed

nadove-ucsc force-pushed the issues/noah-aviel-dove/1498-unit-tests-special-region branch from 6de519f to 8ee14c4 Compare May 1, 2020 20:52

nadove-ucsc requested a review from hannes-ucsc May 1, 2020 21:18

nadove-ucsc assigned hannes-ucsc May 1, 2020

hannes-ucsc approved these changes May 2, 2020

View reviewed changes

hannes-ucsc added the hold warm [process] PR can't land just yet but needs to be rebased daily label May 2, 2020

hannes-ucsc force-pushed the issues/noah-aviel-dove/1498-unit-tests-special-region branch from 8ee14c4 to fe45f8c Compare May 5, 2020 00:35

hannes-ucsc added sandbox [process] Resolution is being verified in sandbox deployment and removed hold warm [process] PR can't land just yet but needs to be rebased daily labels May 5, 2020

Use exotic AWS region for unit tests (#1498)

6722202

hannes-ucsc force-pushed the issues/noah-aviel-dove/1498-unit-tests-special-region branch from fe45f8c to 6722202 Compare May 5, 2020 01:14

hannes-ucsc merged commit 1dd6f80 into develop May 5, 2020

hannes-ucsc deleted the issues/noah-aviel-dove/1498-unit-tests-special-region branch May 5, 2020 01:15

hannes-ucsc removed their assignment May 29, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mock AWS_DEFAULT_REGION for all unit tests #1710

Mock AWS_DEFAULT_REGION for all unit tests #1710

nadove-ucsc commented Apr 16, 2020

codecov bot commented Apr 16, 2020 •

edited

Loading

coveralls commented Apr 16, 2020 •

edited

Loading

jessebrennan left a comment

hannes-ucsc Apr 16, 2020 •

edited

Loading

nadove-ucsc Apr 17, 2020

hannes-ucsc Apr 17, 2020

nadove-ucsc Apr 17, 2020

nadove-ucsc Apr 17, 2020

nadove-ucsc Apr 17, 2020

nadove-ucsc Apr 17, 2020

hannes-ucsc Apr 17, 2020

hannes-ucsc Apr 17, 2020

nadove-ucsc Apr 17, 2020 •

edited

Loading

nadove-ucsc Apr 17, 2020

hannes-ucsc Apr 24, 2020

hannes-ucsc commented Apr 24, 2020 •

edited

Loading

hannes-ucsc left a comment

nadove-ucsc commented Apr 30, 2020

hannes-ucsc commented Apr 30, 2020

hannes-ucsc May 1, 2020

	_aws_region_mock = patch.dict(os.environ, AWS_DEFAULT_REGION='ap-south-1')
	_aws_region_mock = patch.dict(os.environ, AWS_DEFAULT_REGION='us-gov-west-1')

	# Moto will verify that region is valid.
	# We can't use an artificial region name since Moto validates it.

Mock AWS_DEFAULT_REGION for all unit tests #1710

Mock AWS_DEFAULT_REGION for all unit tests #1710

Conversation

nadove-ucsc commented Apr 16, 2020

codecov bot commented Apr 16, 2020 • edited Loading

Codecov Report

coveralls commented Apr 16, 2020 • edited Loading

Pull Request Test Coverage Report for Build 8098

💛 - Coveralls

jessebrennan left a comment

Choose a reason for hiding this comment

hannes-ucsc Apr 16, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nadove-ucsc Apr 17, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hannes-ucsc commented Apr 24, 2020 • edited Loading

hannes-ucsc left a comment

Choose a reason for hiding this comment

nadove-ucsc commented Apr 30, 2020

hannes-ucsc commented Apr 30, 2020

Choose a reason for hiding this comment

codecov bot commented Apr 16, 2020 •

edited

Loading

coveralls commented Apr 16, 2020 •

edited

Loading

hannes-ucsc Apr 16, 2020 •

edited

Loading

nadove-ucsc Apr 17, 2020 •

edited

Loading

hannes-ucsc commented Apr 24, 2020 •

edited

Loading