From bad06746513dbfb3ed2268708f622afc87545e81 Mon Sep 17 00:00:00 2001 From: Jay R Bolton Date: Tue, 16 Feb 2021 14:55:40 -0800 Subject: [PATCH 1/7] Update versions of all requirements --- dev-requirements.txt | 11 +++++------ requirements.txt | 14 +++++++------- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/dev-requirements.txt b/dev-requirements.txt index 87723e8..1fb65d2 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -1,7 +1,6 @@ -mypy==0.610 -bandit==1.4.0 +mypy==0.800 +bandit==1.7.0 mccabe==0.6.1 -pyflakes==2.0.0 -flake8==3.5.0 -grequests==0.3.0 -coverage==4.5.1 +flake8==3.8.4 +grequests==0.6.0 +coverage==5.4 diff --git a/requirements.txt b/requirements.txt index 0d1acdf..23836db 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,8 +1,8 @@ -minio==4.0.11 -Flask==1.0.2 -gunicorn==19.9.0 -gevent==1.4.0 -simplejson==3.16.0 -python-dotenv==0.10.1 -requests==2.21.0 +minio==7.0.2 +Flask==1.1.2 +gunicorn==20.0.4 +gevent==21.1.2 +simplejson==3.17.2 +python-dotenv==0.15.0 +requests==2.25.1 docopt==0.6.2 From c1bc1154dd5cff552507a8ce50470fafbc82d25c Mon Sep 17 00:00:00 2001 From: Jay R Bolton Date: Tue, 16 Feb 2021 14:55:50 -0800 Subject: [PATCH 2/7] Only run bandit on source code, not tests --- scripts/run_tests.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/run_tests.sh b/scripts/run_tests.sh index fe721e3..dfa65fa 100644 --- a/scripts/run_tests.sh +++ b/scripts/run_tests.sh @@ -5,5 +5,5 @@ set -e flake8 --max-complexity 6 src/caching_service flake8 src/test mypy --ignore-missing-imports src -bandit -r src +bandit -r src/caching_service python -m unittest discover src/test/caching_service From 4c586da3c5d1074ab9616b521cf2bb81a5474f43 Mon Sep 17 00:00:00 2001 From: Jay R Bolton Date: Tue, 16 Feb 2021 14:56:08 -0800 Subject: [PATCH 3/7] Fix tests and minio error classes --- src/caching_service/api/api_v1.py | 2 - src/caching_service/minio.py | 27 +++++--- src/test/caching_service/test_api_v1.py | 4 +- src/test/caching_service/test_minio.py | 7 +- src/test/mock_auth/endpoints.json | 89 ------------------------- src/test/mock_auth/invalid_token.json | 23 +++++++ src/test/mock_auth/missing_token.json | 20 ++++++ src/test/mock_auth/non_admin.json | 18 +++++ src/test/mock_auth/non_admin2.json | 22 ++++++ 9 files changed, 106 insertions(+), 106 deletions(-) delete mode 100644 src/test/mock_auth/endpoints.json create mode 100644 src/test/mock_auth/invalid_token.json create mode 100644 src/test/mock_auth/missing_token.json create mode 100644 src/test/mock_auth/non_admin.json create mode 100644 src/test/mock_auth/non_admin2.json diff --git a/src/caching_service/api/api_v1.py b/src/caching_service/api/api_v1.py index 1ca0af3..761fa2d 100644 --- a/src/caching_service/api/api_v1.py +++ b/src/caching_service/api/api_v1.py @@ -2,7 +2,6 @@ import tempfile import json import flask -import minio.error import shutil from ..authorization.service_token import requires_service_token @@ -88,7 +87,6 @@ def delete(cache_id): # -------------- @api_v1.errorhandler(exceptions.MissingCache) -@api_v1.errorhandler(minio.error.NoSuchKey) def missing_cache_file(err): """A cache ID was not found, but was expected to exist.""" result = {'status': 'error', 'error': 'Cache ID not found'} diff --git a/src/caching_service/minio.py b/src/caching_service/minio.py index dbf7479..be424fc 100644 --- a/src/caching_service/minio.py +++ b/src/caching_service/minio.py @@ -23,10 +23,11 @@ # Create the bucket if it does not exist try: minio_client.make_bucket(bucket_name) -except minio.error.BucketAlreadyExists: - pass -except minio.error.BucketAlreadyOwnedByYou: - pass +except minio.error.S3Error as err: + # Acceptable errors + errs = ["BucketAlreadyExists", "BucketAlreadyOwnedByYou"] + if err.code not in errs: + raise err def create_placeholder(cache_id, token_id): @@ -46,7 +47,8 @@ def create_placeholder(cache_id, token_id): """ try: return get_metadata(cache_id) - except minio.error.NoSuchKey: + except exceptions.MissingCache: + # Create the cache key seven_days = 604800 # in seconds expiration = str(int(time.time() + seven_days)) metadata = { @@ -63,8 +65,9 @@ def authorize_access(cache_id, token_id): """ Given a cache ID and token ID, authorize that the token has permission to access the cache. - This will raise caching_service.exceptions.UnauthorizedAccess if it is unauthorized. - This will raise minio.error.NoSuchKey if the cache ID does not exist. + Raises: + - caching_service.exceptions.UnauthorizedAccess if it is unauthorized. + - exceptions.MissingCache if the cache ID does not exist. """ metadata = get_metadata(cache_id) existing_token_id = metadata['token_id'] @@ -105,7 +108,7 @@ def expire_entries(): """ print('Checking the expiration of all stored objects..') now = time.time() - objects = minio_client.list_objects_v2(bucket_name) + objects = minio_client.list_objects(bucket_name) removed_count = 0 total_count = 0 for obj in objects: @@ -134,7 +137,13 @@ def delete_cache(cache_id, token_id): def get_metadata(cache_id): """Return the Minio metadata dict for a cache file.""" - orig_metadata = minio_client.stat_object(bucket_name, cache_id).metadata + try: + orig_metadata = minio_client.stat_object(bucket_name, cache_id).metadata + except minio.error.S3Error as err: + # Catch NoSuchKey errors and raise MissingCache + if err.code != "NoSuchKey": + raise err + raise exceptions.MissingCache(cache_id) # The below keys are how metadata gets stored in minio files for 'expiration', 'filename', etc # For example if you set the metadata 'xyz_abc', then minio will store it as 'X-Amz-Meta-Xyz_abc' return { diff --git a/src/test/caching_service/test_api_v1.py b/src/test/caching_service/test_api_v1.py index fd365c0..3d24988 100644 --- a/src/test/caching_service/test_api_v1.py +++ b/src/test/caching_service/test_api_v1.py @@ -7,9 +7,9 @@ import requests from uuid import uuid4 import functools -from minio.error import NoSuchKey import src.caching_service.minio as minio +from src.caching_service.exceptions import MissingCache url = 'http://web:5000/v1' @@ -309,7 +309,7 @@ def test_delete_valid(self): self.assertEqual(resp.status_code, 200, 'Status code is 200') self.assertEqual(json['status'], 'ok', 'Status is "deleted"') # Test that the cache is inaccessible - with self.assertRaises(NoSuchKey): + with self.assertRaises(MissingCache): minio.get_metadata(cache_id) def test_delete_unauthorized_cache(self): diff --git a/src/test/caching_service/test_minio.py b/src/test/caching_service/test_minio.py index a98ac64..95433d0 100644 --- a/src/test/caching_service/test_minio.py +++ b/src/test/caching_service/test_minio.py @@ -4,7 +4,6 @@ import os import io from werkzeug.datastructures import FileStorage -from minio.error import NoSuchKey from uuid import uuid4 import tempfile @@ -73,7 +72,7 @@ def test_cache_delete(self): minio.create_placeholder(cache_id, token_id) minio.delete_cache(cache_id, token_id) tmp_dir = tempfile.mkdtemp() - with self.assertRaises(NoSuchKey): + with self.assertRaises(exceptions.MissingCache): minio.download_cache(cache_id, token_id, tmp_dir) shutil.rmtree(tmp_dir) @@ -103,7 +102,7 @@ def test_missing_cache_upload(self): cache_id = str(uuid4()) file_storage = self.make_test_file_storage(cache_id, token_id) minio.create_placeholder(cache_id, token_id) - with self.assertRaises(NoSuchKey): + with self.assertRaises(exceptions.MissingCache): minio.upload_cache(cache_id + 'x', token_id, file_storage) file_storage.stream.close() @@ -113,7 +112,7 @@ def test_missing_cache_download(self): cache_id = str(uuid4()) minio.create_placeholder(cache_id, token_id) tmp_dir = tempfile.mkdtemp() - with self.assertRaises(NoSuchKey): + with self.assertRaises(exceptions.MissingCache): minio.download_cache(cache_id + 'x', token_id, tmp_dir) shutil.rmtree(tmp_dir) diff --git a/src/test/mock_auth/endpoints.json b/src/test/mock_auth/endpoints.json deleted file mode 100644 index 1363b81..0000000 --- a/src/test/mock_auth/endpoints.json +++ /dev/null @@ -1,89 +0,0 @@ -[ - { - "methods": [ - "GET" - ], - "path": "/api/V2/token", - "headers": { - "Authorization": "non_admin_token" - }, - "response": { - "status": "200", - "body": { - "type": "Developer", - "id": "xyz-abc-123", - "expires": 1556479867969, - "created": 1548703867969, - "name": "token_name", - "user": "username", - "custom": {}, - "cachefor": 300000 - } - } - }, - { - "methods": [ - "GET" - ], - "path": "/api/V2/token", - "headers": { - "Authorization": "non_admin_token2" - }, - "response": { - "status": "200", - "body": { - "type": "Developer", - "id": "xyz-abc-123-456", - "expires": 1556479867969, - "created": 1548703867969, - "name": "token_name", - "user": "username2", - "custom": {}, - "cachefor": 300000 - } - } - }, - { - "methods": [ - "GET" - ], - "path": "/api/V2/token", - "headers": { - "Authorization": "invalid_token" - }, - "response": { - "status": "401", - "body": { - "error": { - "httpcode": 401, - "httpstatus": "Unauthorized", - "appcode": 10020, - "apperror": "Invalid token", - "message": "10020 Invalid token", - "callid": "1757210147564211", - "time": 1542737889450 - } - } - } - }, - { - "methods": [ - "GET" - ], - "path": "/api/V2/token", - "response": { - "status": "400", - "body": { - "error": { - "httpcode": 400, - "httpstatus": "Bad Request", - "appcode": 10010, - "apperror": "No authentication token", - "message": "10010 No authentication token: No user token provided", - "callid": "7334881776774415", - "time": 1542737656377 - } - } - } - } -] diff --git a/src/test/mock_auth/invalid_token.json b/src/test/mock_auth/invalid_token.json new file mode 100644 index 0000000..aca0046 --- /dev/null +++ b/src/test/mock_auth/invalid_token.json @@ -0,0 +1,23 @@ +{ + "methods": [ + "GET" + ], + "path": "/api/V2/token", + "headers": { + "Authorization": "invalid_token" + }, + "response": { + "status": "401", + "body": { + "error": { + "httpcode": 401, + "httpstatus": "Unauthorized", + "appcode": 10020, + "apperror": "Invalid token", + "message": "10020 Invalid token", + "callid": "1757210147564211", + "time": 1542737889450 + } + } + } +} diff --git a/src/test/mock_auth/missing_token.json b/src/test/mock_auth/missing_token.json new file mode 100644 index 0000000..f2f0ce3 --- /dev/null +++ b/src/test/mock_auth/missing_token.json @@ -0,0 +1,20 @@ +{ + "methods": [ + "GET" + ], + "path": "/api/V2/token", + "response": { + "status": "400", + "body": { + "error": { + "httpcode": 400, + "httpstatus": "Bad Request", + "appcode": 10010, + "apperror": "No authentication token", + "message": "10010 No authentication token: No user token provided", + "callid": "7334881776774415", + "time": 1542737656377 + } + } + } +} diff --git a/src/test/mock_auth/non_admin.json b/src/test/mock_auth/non_admin.json new file mode 100644 index 0000000..0eb5c61 --- /dev/null +++ b/src/test/mock_auth/non_admin.json @@ -0,0 +1,18 @@ +{ + "methods": [ "GET" ], + "path": "/api/V2/token", + "headers": { "Authorization": "non_admin_token" }, + "response": { + "status": "200", + "body": { + "type": "Developer", + "id": "xyz-abc-123", + "expires": 1556479867969, + "created": 1548703867969, + "name": "token_name", + "user": "username", + "custom": {}, + "cachefor": 300000 + } + } +} diff --git a/src/test/mock_auth/non_admin2.json b/src/test/mock_auth/non_admin2.json new file mode 100644 index 0000000..0807228 --- /dev/null +++ b/src/test/mock_auth/non_admin2.json @@ -0,0 +1,22 @@ +{ + "methods": [ + "GET" + ], + "path": "/api/V2/token", + "headers": { + "Authorization": "non_admin_token2" + }, + "response": { + "status": "200", + "body": { + "type": "Developer", + "id": "xyz-abc-123-456", + "expires": 1556479867969, + "created": 1548703867969, + "name": "token_name", + "user": "username2", + "custom": {}, + "cachefor": 300000 + } + } +} From efbb8fab71532a80031a60835e844f6f45103022 Mon Sep 17 00:00:00 2001 From: Jay R Bolton Date: Tue, 16 Feb 2021 15:12:42 -0800 Subject: [PATCH 4/7] Debug travis --- src/test/caching_service/test_api_v1.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/caching_service/test_api_v1.py b/src/test/caching_service/test_api_v1.py index 3d24988..39f53db 100644 --- a/src/test/caching_service/test_api_v1.py +++ b/src/test/caching_service/test_api_v1.py @@ -80,6 +80,7 @@ def test_invalid_auth(self): resp = requests.post(req_data['url'], headers={'Authorization': 'invalid_token'}) json = resp.json() self.assertEqual(resp.status_code, 403, 'Status code is 403') + print('json 83', json) self.assertEqual(json['status'], 'error', 'Status is set to "error"') self.assertTrue('Invalid token' in json['error'], 'Error message is set') @@ -128,6 +129,7 @@ def test_make_cache_id_unauthorized(self): ) json = resp.json() self.assertEqual(resp.status_code, 403, 'Status code is 403') + print('json 132', json) self.assertEqual(json['status'], 'error', 'Status is set to "error"') self.assertTrue('Invalid token' in json['error'], 'Gives error message') From 72e10c7e96d2bb645ea5a00b760fda9e3937bde7 Mon Sep 17 00:00:00 2001 From: Jay R Bolton Date: Tue, 16 Feb 2021 21:23:18 -0800 Subject: [PATCH 5/7] Clean up some dependency boot ordering --- docker-compose.yaml | 9 ++++-- scripts/start_server.sh | 16 +++++----- src/caching_service/minio.py | 42 ++++++++++++++++++++----- src/caching_service/server.py | 2 +- src/caching_service/utils/init_app.py | 16 ++++++++++ src/test/caching_service/test_api_v1.py | 2 -- src/test/mock_auth/missing_token.json | 1 + 7 files changed, 66 insertions(+), 22 deletions(-) create mode 100644 src/caching_service/utils/init_app.py diff --git a/docker-compose.yaml b/docker-compose.yaml index 305b729..0ddbd8c 100644 --- a/docker-compose.yaml +++ b/docker-compose.yaml @@ -1,4 +1,4 @@ -version: '3' +version: '3.8' services: @@ -17,12 +17,15 @@ services: - "127.0.0.1:5000:5000" volumes: - .:/app + depends_on: + - minio + - auth # For running the file server minio: image: minio/minio - ports: - - "127.0.0.1:9000:9000" + expose: + - "9000" environment: - MINIO_ACCESS_KEY=minio - MINIO_SECRET_KEY=minio123 diff --git a/scripts/start_server.sh b/scripts/start_server.sh index 107df73..9775043 100644 --- a/scripts/start_server.sh +++ b/scripts/start_server.sh @@ -7,11 +7,11 @@ calc_workers="$(($(nproc) * 2 + 1))" # Use the WORKERS environment variable, if present workers=${WORKERS:-$calc_workers} -gunicorn \ - --worker-class gevent \ - --timeout 1800 \ - --workers $workers \ - --bind :5000 \ - ${DEVELOPMENT:+"--reload"} \ - src.caching_service.server:app - app:app +python -m src.caching_service.utils.init_app && \ + gunicorn \ + --worker-class gevent \ + --timeout 1800 \ + --workers $workers \ + --bind :5000 \ + ${DEVELOPMENT:+"--reload"} \ + src.caching_service.server:app diff --git a/src/caching_service/minio.py b/src/caching_service/minio.py index be424fc..530260d 100644 --- a/src/caching_service/minio.py +++ b/src/caching_service/minio.py @@ -4,6 +4,7 @@ import tempfile import os import io +import requests import shutil from werkzeug.utils import secure_filename @@ -20,14 +21,39 @@ ) bucket_name = Config.minio_bucket_name -# Create the bucket if it does not exist -try: - minio_client.make_bucket(bucket_name) -except minio.error.S3Error as err: - # Acceptable errors - errs = ["BucketAlreadyExists", "BucketAlreadyOwnedByYou"] - if err.code not in errs: - raise err + +def initialize_bucket(): + """ + Create the default bucket if it does not exist + """ + try: + print('making bucket', bucket_name) + minio_client.make_bucket(bucket_name) + print('done making bucket', bucket_name) + except minio.error.S3Error as err: + # Acceptable errors + errs = ["BucketAlreadyExists", "BucketAlreadyOwnedByYou"] + if err.code not in errs: + raise err + + +def wait_for_service(): + """ + Wait for the minio service to be healthy + """ + url = f'http://{Config.minio_host}/minio/health/live' + max_time = 180 + start = time.time() + while True: + try: + requests.get(url).raise_for_status() + print("Minio is healthy! Continuing.") + break + except Exception as err: + if time.time() > start + max_time: + raise RuntimeError("Timed out waiting for Minio") + print(f"Still waiting for Minio at {url} to be healthy:") + print(err) def create_placeholder(cache_id, token_id): diff --git a/src/caching_service/server.py b/src/caching_service/server.py index f0f1409..9deb051 100644 --- a/src/caching_service/server.py +++ b/src/caching_service/server.py @@ -9,11 +9,11 @@ from .exceptions import MissingHeader, InvalidContentType, UnauthorizedAccess from .config import Config +# Initialize the server app = flask.Flask(__name__) app.config['DEBUG'] = os.environ.get('DEVELOPMENT') app.config['SECRET_KEY'] = Config.secret_key app.url_map.strict_slashes = False # allow both `get /v1/` and `get /v1` - app.register_blueprint(api_v1, url_prefix='/v1') diff --git a/src/caching_service/utils/init_app.py b/src/caching_service/utils/init_app.py new file mode 100644 index 0000000..775fffc --- /dev/null +++ b/src/caching_service/utils/init_app.py @@ -0,0 +1,16 @@ +""" +Any initialization code that needs to be run before any workers start: + - wait for Minio to be healthy + - create the bucket +""" +import src.caching_service.minio as minio + + +def init_app(): + # Wait for minio to be healthy + minio.wait_for_service() + minio.initialize_bucket() + + +if __name__ == '__main__': + init_app() diff --git a/src/test/caching_service/test_api_v1.py b/src/test/caching_service/test_api_v1.py index 39f53db..3d24988 100644 --- a/src/test/caching_service/test_api_v1.py +++ b/src/test/caching_service/test_api_v1.py @@ -80,7 +80,6 @@ def test_invalid_auth(self): resp = requests.post(req_data['url'], headers={'Authorization': 'invalid_token'}) json = resp.json() self.assertEqual(resp.status_code, 403, 'Status code is 403') - print('json 83', json) self.assertEqual(json['status'], 'error', 'Status is set to "error"') self.assertTrue('Invalid token' in json['error'], 'Error message is set') @@ -129,7 +128,6 @@ def test_make_cache_id_unauthorized(self): ) json = resp.json() self.assertEqual(resp.status_code, 403, 'Status code is 403') - print('json 132', json) self.assertEqual(json['status'], 'error', 'Status is set to "error"') self.assertTrue('Invalid token' in json['error'], 'Gives error message') diff --git a/src/test/mock_auth/missing_token.json b/src/test/mock_auth/missing_token.json index f2f0ce3..1b43f84 100644 --- a/src/test/mock_auth/missing_token.json +++ b/src/test/mock_auth/missing_token.json @@ -2,6 +2,7 @@ "methods": [ "GET" ], + "absent_headers": ["Authorization"], "path": "/api/V2/token", "response": { "status": "400", From ed105add6be372b31d658cba9155f284aa962bf9 Mon Sep 17 00:00:00 2001 From: Jay R Bolton Date: Tue, 16 Feb 2021 21:48:45 -0800 Subject: [PATCH 6/7] Try a docker-compose version that works on travis --- docker-compose.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker-compose.yaml b/docker-compose.yaml index 0ddbd8c..fe3f61b 100644 --- a/docker-compose.yaml +++ b/docker-compose.yaml @@ -1,4 +1,4 @@ -version: '3.8' +version: '3.7' services: From 65b8395233ca7e8826581fb8c3fc67922e7e9016 Mon Sep 17 00:00:00 2001 From: Jay R Bolton Date: Thu, 18 Feb 2021 13:17:48 -0800 Subject: [PATCH 7/7] Clean up logging statement --- src/caching_service/minio.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/caching_service/minio.py b/src/caching_service/minio.py index 530260d..5ee5b7f 100644 --- a/src/caching_service/minio.py +++ b/src/caching_service/minio.py @@ -27,9 +27,9 @@ def initialize_bucket(): Create the default bucket if it does not exist """ try: - print('making bucket', bucket_name) + print(f"Making bucket with name '{bucket_name}'") minio_client.make_bucket(bucket_name) - print('done making bucket', bucket_name) + print(f"Done making bucket '{bucket_name}'") except minio.error.S3Error as err: # Acceptable errors errs = ["BucketAlreadyExists", "BucketAlreadyOwnedByYou"]