From 2a259fa9fcf4731c33491be56309b363bf7316b9 Mon Sep 17 00:00:00 2001 From: Nathan Gillett Date: Wed, 16 Sep 2020 15:50:55 -0400 Subject: [PATCH] Implement environment support in upload API [RHELDST-2188] This commit removes the 'bucket' parameter from the s3 upload API and it's endpoints, replacing it with 'env'. The bucket name is now generated to match the corresponding environment defined in exodus-gw-config.ini. --- docs/api-guide.rst | 2 +- exodus-gw-config.ini | 19 +++++++++++++++ exodus_gw/s3/api.py | 48 ++++++++++++++++++------------------- exodus_gw/s3/util.py | 12 ++++++++++ exodus_gw/settings.py | 10 ++++++++ tests/s3/test_manage_mpu.py | 22 ++++++++--------- tests/s3/test_upload.py | 40 +++++++++++++++++++++++++++---- 7 files changed, 112 insertions(+), 41 deletions(-) create mode 100644 exodus-gw-config.ini diff --git a/docs/api-guide.rst b/docs/api-guide.rst index 33cda5b6..4669cff5 100644 --- a/docs/api-guide.rst +++ b/docs/api-guide.rst @@ -61,7 +61,7 @@ Usage: config=Config(client_cert=('client.crt', 'client.key'))) # Basic APIs such as upload_file now work as usual - bucket = s3.Bucket('exodus-cdn-dev') + bucket = s3.Bucket('dev') bucket.upload_file('/tmp/hello.txt', 'aec070645fe53ee3b3763059376134f058cc337247c978add178b6ccdfb0019f') diff --git a/exodus-gw-config.ini b/exodus-gw-config.ini new file mode 100644 index 00000000..0012b7fe --- /dev/null +++ b/exodus-gw-config.ini @@ -0,0 +1,19 @@ +# Configuration for the exodus-gw application + + +# qa currently shares storage with dev +[qa] +bucket = exodus-cdn-dev +table = exodus-cdn-dev + +[dev] +bucket = exodus-cdn-dev +table = exodus-cdn-dev + +[stage] +bucket = exodus-cdn-stage +table = exodus-cdn-stage + +[prod] +bucket = exodus-cdn-prod +table = exodus-cdn-prod diff --git a/exodus_gw/s3/api.py b/exodus_gw/s3/api.py index babfc3b2..9cedb29a 100644 --- a/exodus_gw/s3/api.py +++ b/exodus_gw/s3/api.py @@ -38,7 +38,7 @@ from ..app import app -from .util import extract_mpu_parts, xml_response, RequestReader +from .util import extract_mpu_parts, xml_response, RequestReader, bucket_name LOG = logging.getLogger("s3") @@ -47,10 +47,6 @@ # - format of 'key' should be enforced (sha256sum) # - a way to check if object is already uploaded, e.g. HEAD # - limits on chunk sizes during multipart upload should be decided and enforced -# - should support configuration of the target S3 environment rather than using -# whatever's the boto default -# - {bucket} shouldn't be a parameter, as the target bucket for each exodus env -# is predefined and not controlled by the caller. # - requests should be authenticated @@ -64,14 +60,14 @@ def s3_client(): @app.post( - "/upload/{bucket}/{key}", + "/upload/{env}/{key}", tags=["upload"], summary="Create/complete multipart upload", response_class=Response, ) async def multipart_upload( request: Request, - bucket: str = Path(..., description="S3 bucket name"), + env: str = Path(..., description="Target CDN environment"), key: str = Path(..., description="S3 object key"), uploadId: Optional[str] = Query( None, @@ -99,7 +95,7 @@ async def multipart_upload( """Create or complete a multi-part upload. To create a multi-part upload: - - include ``uploads`` in query string, with no value (e.g. ``POST /upload/{bucket}/{key}?uploads``) + - include ``uploads`` in query string, with no value (e.g. ``POST /upload/{env}/{key}?uploads``) - see also: https://docs.aws.amazon.com/AmazonS3/latest/API/API_CreateMultipartUpload.html To complete a multi-part upload: @@ -110,11 +106,11 @@ async def multipart_upload( if uploads == "": # Means a new upload is requested - return await create_multipart_upload(bucket, key) + return await create_multipart_upload(env, key) elif uploads is None and uploadId: # Given an existing upload to complete - return await complete_multipart_upload(bucket, key, uploadId, request) + return await complete_multipart_upload(env, key, uploadId, request) # Caller did something wrong raise HTTPException( @@ -125,14 +121,14 @@ async def multipart_upload( @app.put( - "/upload/{bucket}/{key}", + "/upload/{env}/{key}", tags=["upload"], summary="Upload bytes", response_class=Response, ) async def upload( request: Request, - bucket: str = Path(..., description="S3 bucket name"), + env: str = Path(..., description="Target CDN environment"), key: str = Path(..., description="S3 object key"), uploadId: Optional[str] = Query( None, description="ID of an existing multi-part upload." @@ -157,19 +153,19 @@ async def upload( if uploadId is None and partNumber is None: # Single-part upload - return await object_put(bucket, key, request) + return await object_put(env, key, request) # Multipart upload - return await multipart_put(bucket, key, uploadId, partNumber, request) + return await multipart_put(env, key, uploadId, partNumber, request) -async def object_put(bucket: str, key: str, request: Request): +async def object_put(env: str, key: str, request: Request): # Single-part upload handler: entire object is written via one PUT. reader = RequestReader.get_reader(request) async with s3_client() as s3: response = await s3.put_object( - Bucket=bucket, + Bucket=bucket_name(env), Key=key, Body=reader, ContentMD5=request.headers["Content-MD5"], @@ -180,7 +176,7 @@ async def object_put(bucket: str, key: str, request: Request): async def complete_multipart_upload( - bucket: str, key: str, uploadId: str, request: Request + env: str, key: str, uploadId: str, request: Request ): body = await request.body() @@ -190,7 +186,7 @@ async def complete_multipart_upload( async with s3_client() as s3: response = await s3.complete_multipart_upload( - Bucket=bucket, + Bucket=bucket_name(env), Key=key, UploadId=uploadId, MultipartUpload={"Parts": parts}, @@ -206,9 +202,11 @@ async def complete_multipart_upload( ) -async def create_multipart_upload(bucket: str, key: str): +async def create_multipart_upload(env: str, key: str): async with s3_client() as s3: - response = await s3.create_multipart_upload(Bucket=bucket, Key=key) + response = await s3.create_multipart_upload( + Bucket=bucket_name(env), Key=key + ) return xml_response( "CreateMultipartUploadOutput", @@ -219,14 +217,14 @@ async def create_multipart_upload(bucket: str, key: str): async def multipart_put( - bucket: str, key: str, uploadId: str, partNumber: int, request: Request + env: str, key: str, uploadId: str, partNumber: int, request: Request ): reader = RequestReader.get_reader(request) async with s3_client() as s3: response = await s3.upload_part( Body=reader, - Bucket=bucket, + Bucket=bucket_name(env), Key=key, PartNumber=partNumber, UploadId=uploadId, @@ -238,14 +236,14 @@ async def multipart_put( @app.delete( - "/upload/{bucket}/{key}", + "/upload/{env}/{key}", tags=["upload"], summary="Abort multipart upload", response_description="Empty response", response_class=Response, ) async def abort_multipart_upload( - bucket: str = Path(..., description="S3 bucket name"), + env: str = Path(..., description="Target CDN environment"), key: str = Path(..., description="S3 object key"), uploadId: str = Query(..., description="ID of a multipart upload"), ): @@ -260,7 +258,7 @@ async def abort_multipart_upload( async with s3_client() as s3: await s3.abort_multipart_upload( - Bucket=bucket, Key=key, UploadId=uploadId + Bucket=bucket_name(env), Key=key, UploadId=uploadId ) return Response() diff --git a/exodus_gw/s3/util.py b/exodus_gw/s3/util.py index 15246e5b..365f3a91 100644 --- a/exodus_gw/s3/util.py +++ b/exodus_gw/s3/util.py @@ -3,6 +3,7 @@ from fastapi import Response from defusedxml.ElementTree import fromstring +from ..settings import exodus_gw_config def extract_mpu_parts( @@ -94,3 +95,14 @@ def get_reader(cls, request): # a helper to make tests easier to write. # tests can patch over this to effectively disable streaming. return cls(request) + + +def bucket_name(env: str): + """Produces s3 bucket name for the given environment name.""" + + config = exodus_gw_config() + + if env in config.sections() and "bucket" in config.options(env): + return config.get(env, "bucket") + + raise ValueError("Environment '%s' not supported" % env) diff --git a/exodus_gw/settings.py b/exodus_gw/settings.py index f978ba51..70fe1432 100644 --- a/exodus_gw/settings.py +++ b/exodus_gw/settings.py @@ -1,6 +1,16 @@ from functools import lru_cache from pydantic import BaseSettings +import configparser + + +def exodus_gw_config(file_name: str = "exodus-gw-config.ini"): + """Parse and return exodus-gw configuration""" + + config = configparser.ConfigParser() + config.read(file_name) + + return config class Settings(BaseSettings): diff --git a/tests/s3/test_manage_mpu.py b/tests/s3/test_manage_mpu.py index 82ecc1d1..e639aa2d 100644 --- a/tests/s3/test_manage_mpu.py +++ b/tests/s3/test_manage_mpu.py @@ -15,21 +15,21 @@ async def test_create_mpu(mock_s3_client): """Creating a multipart upload is delegated correctly to S3.""" mock_s3_client.create_multipart_upload.return_value = { - "Bucket": "my-bucket", + "Bucket": "exodus-cdn-dev", "Key": TEST_KEY, "UploadId": "my-great-upload", } response = await multipart_upload( None, - bucket="my-bucket", + env="dev", key=TEST_KEY, uploads="", ) # It should delegate request to real S3 mock_s3_client.create_multipart_upload.assert_called_once_with( - Bucket="my-bucket", + Bucket="exodus-cdn-dev", Key=TEST_KEY, ) @@ -42,7 +42,7 @@ async def test_create_mpu(mock_s3_client): # It should include the appropriate data expected = xml_response( "CreateMultipartUploadOutput", - Bucket="my-bucket", + Bucket="exodus-cdn-dev", Key=TEST_KEY, UploadId="my-great-upload", ).body @@ -55,7 +55,7 @@ async def test_complete_mpu(mock_s3_client): mock_s3_client.complete_multipart_upload.return_value = { "Location": "https://example.com/some-object", - "Bucket": "my-bucket", + "Bucket": "exodus-cdn-dev", "Key": TEST_KEY, "ETag": "my-better-etag", } @@ -80,7 +80,7 @@ async def test_complete_mpu(mock_s3_client): response = await multipart_upload( request=request, - bucket="my-bucket", + env="dev", key=TEST_KEY, uploadId="my-better-upload", uploads=None, @@ -88,7 +88,7 @@ async def test_complete_mpu(mock_s3_client): # It should delegate request to real S3 mock_s3_client.complete_multipart_upload.assert_called_once_with( - Bucket="my-bucket", + Bucket="exodus-cdn-dev", Key=TEST_KEY, UploadId="my-better-upload", MultipartUpload={ @@ -109,7 +109,7 @@ async def test_complete_mpu(mock_s3_client): expected = xml_response( "CompleteMultipartUploadOutput", Location="https://example.com/some-object", - Bucket="my-bucket", + Bucket="exodus-cdn-dev", Key=TEST_KEY, ETag="my-better-etag", ).body @@ -123,7 +123,7 @@ async def test_bad_mpu_call(): with pytest.raises(HTTPException) as exc_info: await multipart_upload( request=None, - bucket="my-bucket", + env="dev", key=TEST_KEY, uploadId="oops", uploads="not valid to mix these args", @@ -137,14 +137,14 @@ async def test_abort_mpu(mock_s3_client): """Aborting a multipart upload is correctly delegated to S3.""" response = await abort_multipart_upload( - bucket="my-bucket", + env="dev", key=TEST_KEY, uploadId="my-lame-upload", ) # It should delegate the request to real S3 mock_s3_client.abort_multipart_upload.assert_called_once_with( - Bucket="my-bucket", + Bucket="exodus-cdn-dev", Key=TEST_KEY, UploadId="my-lame-upload", ) diff --git a/tests/s3/test_upload.py b/tests/s3/test_upload.py index 28b012ad..8dd17144 100644 --- a/tests/s3/test_upload.py +++ b/tests/s3/test_upload.py @@ -46,7 +46,7 @@ async def test_full_upload(mock_s3_client, mock_request_reader): response = await upload( request=request, - bucket="my-bucket", + env="dev", key=TEST_KEY, uploadId=None, partNumber=None, @@ -54,7 +54,7 @@ async def test_full_upload(mock_s3_client, mock_request_reader): # It should delegate request to real S3 mock_s3_client.put_object.assert_called_once_with( - Bucket="my-bucket", + Bucket="exodus-cdn-dev", Key=TEST_KEY, Body=b"some bytes", ContentMD5="9d0568469d206c1aedf1b71f12f474bc", @@ -89,7 +89,7 @@ async def test_part_upload(mock_s3_client, mock_request_reader): response = await upload( request=request, - bucket="my-bucket", + env="dev", key=TEST_KEY, uploadId="my-best-upload", partNumber=88, @@ -97,7 +97,7 @@ async def test_part_upload(mock_s3_client, mock_request_reader): # It should delegate request to real S3 mock_s3_client.upload_part.assert_called_once_with( - Bucket="my-bucket", + Bucket="exodus-cdn-dev", Key=TEST_KEY, Body=b"best bytes", PartNumber=88, @@ -114,3 +114,35 @@ async def test_part_upload(mock_s3_client, mock_request_reader): # It should have an empty body assert response.body == b"" + + +@pytest.mark.asyncio +async def test_upload_invalid_env(mock_s3_client, mock_request_reader): + """Uploading to an invalid environment does not delegate to s3.""" + + mock_s3_client.put_object.return_value = { + "ETag": "a1b2c3", + } + + request = mock.Mock( + headers={ + "Content-MD5": "9d0568469d206c1aedf1b71f12f474bc", + "Content-Length": "10", + } + ) + mock_request_reader.return_value = b"some bytes" + + with pytest.raises(ValueError) as err: + await upload( + request=request, + env="test", + key=TEST_KEY, + uploadId=None, + partNumber=None, + ) + + # It should not delegate request to real S3 + assert not mock_s3_client.put_object.called + + # It should produce an error message + assert str(err.value) == "Environment 'test' not supported"