Skip to content

Commit

Permalink
Fix supported list of intrinsic upload path styles
Browse files Browse the repository at this point in the history
**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.
  • Loading branch information
sbkok committed Jun 19, 2020
1 parent ad47b0e commit 3bee88d
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,15 @@
from mock import Mock
from s3 import S3


@fixture
def us_east_1_cls():
return S3(
'us-east-1',
'some_bucket'
)


@fixture
def eu_west_1_cls():
cls = S3(
Expand All @@ -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) == \
Expand All @@ -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) == \
Expand All @@ -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) == \
Expand All @@ -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) == \
Expand All @@ -57,20 +83,23 @@ 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) == \
"{key}".format(
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) == \
"{key}".format(
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) == \
Expand All @@ -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) == \
Expand All @@ -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) == \
Expand All @@ -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) == \
Expand All @@ -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'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(':')
Expand Down

0 comments on commit 3bee88d

Please sign in to comment.