From 3bee88d9fcb5539cccbb3e2737778820677540f6 Mon Sep 17 00:00:00 2001 From: Simon Kok Date: Thu, 18 Jun 2020 15:02:25 +0200 Subject: [PATCH] Fix supported list of intrinsic upload path styles **Why?** When the `Resolver` would find a usage of an intrinsic upload, it would check the requested path style against an outdated list of supported path styles. **How did this happen?** The `S3` class implements the path styles, but did not maintain the list of supported paths. While new paths were added to the S3 class, its supported list of paths in the `Resolver` class was not updated. With this change, the supported list of path styles becomes a class method on the `S3` class instead. Such that the list and its implementation are managed by the same class instead. --- .../adf-build/shared/python/s3.py | 16 +++++++++ .../adf-build/shared/python/tests/test_s3.py | 33 +++++++++++++++++++ .../adf-build/shared/resolver.py | 10 +++--- 3 files changed, 55 insertions(+), 4 deletions(-) diff --git a/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/s3.py b/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/s3.py index 639c6f72a..09b3675f5 100644 --- a/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/s3.py +++ b/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/s3.py @@ -22,6 +22,22 @@ def __init__(self, region, bucket): self.resource = boto3.resource('s3', region_name=region) self.bucket = bucket + @staticmethod + def supported_path_styles(): + """ + Fetch the list of supported path styles. + + Returns: + list(str): The list of supported path styles. + """ + return [ + 'path', + 's3-key-only', + 's3-uri', + 's3-url', + 'virtual-hosted', + ] + def build_pathing_style(self, style, key): if style == 's3-url': return "s3://{bucket}/{key}".format( diff --git a/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/tests/test_s3.py b/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/tests/test_s3.py index b8edc4c18..965b86b34 100644 --- a/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/tests/test_s3.py +++ b/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/tests/test_s3.py @@ -10,6 +10,7 @@ from mock import Mock from s3 import S3 + @fixture def us_east_1_cls(): return S3( @@ -17,6 +18,7 @@ def us_east_1_cls(): 'some_bucket' ) + @fixture def eu_west_1_cls(): cls = S3( @@ -25,6 +27,27 @@ def eu_west_1_cls(): ) return cls + +def test_supported_path_styles_path(): + assert 'path' in S3.supported_path_styles() + + +def test_supported_path_styles_s3_key_only(): + assert 's3-key-only' in S3.supported_path_styles() + + +def test_supported_path_styles_s3_uri(): + assert 's3-uri' in S3.supported_path_styles() + + +def test_supported_path_styles_s3_url(): + assert 's3-url' in S3.supported_path_styles() + + +def test_supported_path_styles_virtual_hosted(): + assert 'virtual-hosted' in S3.supported_path_styles() + + def test_build_pathing_style_s3_url_us_east_1(us_east_1_cls): key = 'some/key' assert us_east_1_cls.build_pathing_style('s3-url', key) == \ @@ -33,6 +56,7 @@ def test_build_pathing_style_s3_url_us_east_1(us_east_1_cls): key=key, ) + def test_build_pathing_style_s3_url_any_other_region(eu_west_1_cls): key = 'some/key' assert eu_west_1_cls.build_pathing_style('s3-url', key) == \ @@ -41,6 +65,7 @@ def test_build_pathing_style_s3_url_any_other_region(eu_west_1_cls): key=key, ) + def test_build_pathing_style_s3_uri_us_east_1(us_east_1_cls): key = 'some/key' assert us_east_1_cls.build_pathing_style('s3-uri', key) == \ @@ -49,6 +74,7 @@ def test_build_pathing_style_s3_uri_us_east_1(us_east_1_cls): key=key, ) + def test_build_pathing_style_s3_uri_any_other_region(eu_west_1_cls): key = 'some/key' assert eu_west_1_cls.build_pathing_style('s3-uri', key) == \ @@ -57,6 +83,7 @@ def test_build_pathing_style_s3_uri_any_other_region(eu_west_1_cls): key=key, ) + def test_build_pathing_style_s3_key_only_us_east_1(us_east_1_cls): key = 'some/key' assert us_east_1_cls.build_pathing_style('s3-key-only', key) == \ @@ -64,6 +91,7 @@ def test_build_pathing_style_s3_key_only_us_east_1(us_east_1_cls): key=key, ) + def test_build_pathing_style_s3_key_only_any_other_region(eu_west_1_cls): key = 'some/key' assert eu_west_1_cls.build_pathing_style('s3-key-only', key) == \ @@ -71,6 +99,7 @@ def test_build_pathing_style_s3_key_only_any_other_region(eu_west_1_cls): key=key, ) + def test_build_pathing_style_path_us_east_1(us_east_1_cls): key = 'some/key' assert us_east_1_cls.build_pathing_style('path', key) == \ @@ -79,6 +108,7 @@ def test_build_pathing_style_path_us_east_1(us_east_1_cls): key=key, ) + def test_build_pathing_style_path_any_other_region(eu_west_1_cls): key = 'some/key' assert eu_west_1_cls.build_pathing_style('path', key) == \ @@ -88,6 +118,7 @@ def test_build_pathing_style_path_any_other_region(eu_west_1_cls): key=key, ) + def test_build_pathing_style_virtual_hosted_us_east_1(us_east_1_cls): key = 'some/key' assert us_east_1_cls.build_pathing_style('virtual-hosted', key) == \ @@ -96,6 +127,7 @@ def test_build_pathing_style_virtual_hosted_us_east_1(us_east_1_cls): key=key, ) + def test_build_pathing_style_virtual_hosted_any_other_region(eu_west_1_cls): key = 'some/key' assert eu_west_1_cls.build_pathing_style('virtual-hosted', key) == \ @@ -105,6 +137,7 @@ def test_build_pathing_style_virtual_hosted_any_other_region(eu_west_1_cls): key=key, ) + def test_build_pathing_style_unknown_style(us_east_1_cls): key = 'some/key' style = 'unknown' diff --git a/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/resolver.py b/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/resolver.py index e35091b82..b2c4a066c 100644 --- a/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/resolver.py +++ b/src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/resolver.py @@ -93,11 +93,13 @@ def fetch_stack_output(self, value, key, optional=False): # pylint: disable=too- return True def upload(self, value, key, file_name): - if not any(item in value for item in ['path', 'virtual-hosted', 's3-key-only']): + if not any(item in value for item in S3.supported_path_styles()): raise Exception( - 'When uploading to S3 you need to specify a ' - 'pathing style for the response either path or virtual-hosted, ' - 'read more: https://docs.aws.amazon.com/AmazonS3/latest/dev/VirtualHosting.html' + 'When uploading to S3 you need to specify a path style' + 'to use for the returned value to be used. ' + 'Supported path styles include: {supported_list}'.format( + supported_list=S3.supported_path_styles(), + ) ) from None if str(value).count(':') > 2: [_, region, style, value] = value.split(':')