Merge pull request boto#1048 from JordonPhillips/dual-accelerate

Allow both dualstack and accelerate

botocore/args.py

@@ -20,7 +20,6 @@
 import logging
 
 import botocore.serialize
-from botocore.utils import fix_s3_host
 from botocore.signers import RequestSigner
 from botocore.config import Config
 from botocore.endpoint import EndpointCreator
@@ -59,10 +58,7 @@ def get_client_args(self, service_model, region_name, is_secure,
             endpoint_config['signature_version'],
             credentials, event_emitter)
 
-        # Add any additional s3 configuration for client
         config_kwargs['s3'] = s3_config
-        self._conditionally_unregister_fix_s3_host(endpoint_url, event_emitter)
-
         new_config = Config(**config_kwargs)
         endpoint_creator = EndpointCreator(event_emitter)
 
@@ -177,11 +173,6 @@ def compute_s3_config(self, scoped_config, client_config):
 
         return s3_configuration
 
-    def _conditionally_unregister_fix_s3_host(self, endpoint_url, emitter):
-        # If the user is providing a custom endpoint, we should not alter it.
-        if endpoint_url is not None:
-            emitter.unregister('before-sign.s3', fix_s3_host)
-
     def _convert_config_to_bool(self, config_dict, keys):
         # Make sure any further modifications to this section of the config
         # will not affect the scoped config by making a copy of it.

botocore/client.py

@@ -25,13 +25,14 @@
 from botocore.model import ServiceModel
 from botocore.paginate import Paginator
 from botocore.utils import CachedProperty
-from botocore.utils import fix_s3_host
 from botocore.utils import get_service_module_name
-from botocore.utils import switch_to_virtual_host_style
 from botocore.utils import switch_host_s3_accelerate
-from botocore.utils import S3_ACCELERATE_ENDPOINT
 from botocore.utils import S3RegionRedirector
+from botocore.utils import fix_s3_host
+from botocore.utils import switch_to_virtual_host_style
+from botocore.utils import S3_ACCELERATE_WHITELIST
 from botocore.args import ClientArgsCreator
+from botocore.compat import urlsplit
 # Keep this imported.  There's pre-existing code that uses
 # "from botocore.client import Config".
 from botocore.config import Config
@@ -67,7 +68,7 @@ def create_client(self, service_name, region_name, is_secure=True,
             service_model, region_name, is_secure, endpoint_url,
             verify, credentials, scoped_config, client_config, endpoint_bridge)
         service_client = cls(**client_args)
-        self._create_s3_redirector(service_client, endpoint_bridge)
+        self._register_s3_events(service_client, endpoint_bridge, endpoint_url)
         return service_client
 
     def create_client_class(self, service_name, api_version=None):
@@ -114,10 +115,101 @@ def _register_retries(self, service_model):
         self._event_emitter.register('needs-retry.%s' % endpoint_prefix,
                                      handler, unique_id=unique_id)
 
-    def _create_s3_redirector(self, client, endpoint_bridge):
+    def _register_s3_events(self, client, endpoint_bridge, endpoint_url):
         if client.meta.service_model.service_name != 's3':
             return
         S3RegionRedirector(endpoint_bridge, client).register()
+        self._set_s3_addressing_style(
+            endpoint_url, client.meta.config.s3, client.meta.events)
+        # Enable accelerate if the configuration is set to to true or the
+        # endpoint being used matches one of the accelerate endpoints.
+        if self._is_s3_accelerate(endpoint_url, client.meta.config.s3):
+            # Also make sure that the hostname gets switched to
+            # s3-accelerate.amazonaws.com
+            client.meta.events.register_first(
+                'request-created.s3', switch_host_s3_accelerate)
+
+    def _set_s3_addressing_style(self, endpoint_url, s3_config, event_emitter):
+        if s3_config is None:
+            s3_config = {}
+
+        addressing_style = self._get_s3_addressing_style(
+            endpoint_url, s3_config)
+        handler = self._get_s3_addressing_handler(
+            endpoint_url, s3_config, addressing_style)
+        if handler is not None:
+            event_emitter.register('before-sign.s3', handler)
+
+    def _get_s3_addressing_style(self, endpoint_url, s3_config):
+        # Use virtual host style addressing if accelerate is enabled or if
+        # the given endpoint url is an accelerate endpoint.
+        accelerate = s3_config.get('use_accelerate_endpoint', False)
+        if accelerate or self._is_s3_accelerate(endpoint_url, s3_config):
+            return 'virtual'
+
+        # If a particular addressing style is configured, use it.
+        configured_addressing_style = s3_config.get('addressing_style')
+        if configured_addressing_style:
+            return configured_addressing_style
+
+    def _get_s3_addressing_handler(self, endpoint_url, s3_config,
+                                   addressing_style):
+        # If virtual host style was configured, use it regardless of whether
+        # or not the bucket looks dns compatible.
+        if addressing_style == 'virtual':
+            logger.debug("Using S3 virtual host style addressing.")
+            return switch_to_virtual_host_style
+
+        # If path style is configured, no additional steps are needed. If
+        # endpoint_url was specified, don't default to virtual. We could
+        # potentially default provided endpoint urls to virtual hosted
+        # style, but for now it is avoided.
+        if addressing_style == 'path' or endpoint_url is not None:
+            logger.debug("Using S3 path style addressing.")
+            return None
+
+        logger.debug("Defaulting to S3 virtual host style addressing with "
+                     "path style addressing fallback.")
+
+        # For dual stack mode, we need to clear the default endpoint url in
+        # order to use the existing netloc if the bucket is dns compatible.
+        if s3_config.get('use_dualstack_endpoint', False):
+            return functools.partial(
+                fix_s3_host, default_endpoint_url=None)
+
+        # By default, try to use virtual style with path fallback.
+        return fix_s3_host
+
+    def _is_s3_accelerate(self, endpoint_url, s3_config):
+        # Accelerate has been explicitly configured.
+        if s3_config is not None and s3_config.get('use_accelerate_endpoint'):
+            return True
+
+        # Accelerate mode is turned on automatically if an endpoint url is
+        # provided that matches the accelerate scheme.
+        if endpoint_url is None:
+            return False
+
+        # Accelerate is only valid for Amazon endpoints.
+        netloc = urlsplit(endpoint_url).netloc
+        if not netloc.endswith('amazonaws.com'):
+            return False
+
+        # The first part of the url should always be s3-accelerate.
+        parts = netloc.split('.')
+        if parts[0] != 's3-accelerate':
+            return False
+
+        # Url parts between 's3-accelerate' and 'amazonaws.com' which
+        # represent different url features.
+        feature_parts = parts[1:-2]
+
+        # There should be no duplicate url parts.
+        if len(feature_parts) != len(set(feature_parts)):
+            return False
+
+        # Remaining parts must all be in the whitelist.
+        return all(p in S3_ACCELERATE_WHITELIST for p in feature_parts)
 
     def _get_client_args(self, service_model, region_name, is_secure,
                          endpoint_url, verify, credentials,
@@ -406,55 +498,6 @@ def _register_handlers(self):
                                   self.meta.service_model.endpoint_prefix,
                                   self._request_signer.handler)
 
-        self._register_s3_specific_handlers()
-
-    def _register_s3_specific_handlers(self):
-        # Register all of the s3 specific handlers
-        if self.meta.config.s3 is None:
-            s3_addressing_style = None
-            s3_accelerate = None
-            s3_dualstack = None
-        else:
-            s3_addressing_style = self.meta.config.s3.get('addressing_style')
-            s3_accelerate = self.meta.config.s3.get('use_accelerate_endpoint')
-            s3_dualstack = self.meta.config.s3.get('use_dualstack_endpoint')
-
-        # Enable accelerate if the configuration is set to to true or the
-        # endpoint being used matches one of the Accelerate endpoints.
-        if s3_accelerate or S3_ACCELERATE_ENDPOINT in self._endpoint.host:
-            # Amazon S3 accelerate is being used then always use the virtual
-            # style of addressing because it is required.
-            self._force_virtual_style_s3_addressing()
-            # Also make sure that the hostname gets switched to
-            # s3-accelerate.amazonaws.com
-            self.meta.events.register_first(
-                'request-created.s3', switch_host_s3_accelerate)
-        elif s3_addressing_style:
-            # Otherwise go ahead with the style the user may have specified.
-            if s3_addressing_style == 'path':
-                self._force_path_style_s3_addressing()
-            elif s3_addressing_style == 'virtual':
-                self._force_virtual_style_s3_addressing()
-        elif s3_dualstack:
-            self.meta.events.unregister('before-sign.s3', fix_s3_host)
-            self.meta.events.register(
-                'before-sign.s3',
-                functools.partial(fix_s3_host, default_endpoint_url=None))
-
-    def _force_path_style_s3_addressing(self):
-        # Do not try to modify the host if path is specified. The
-        # ``fix_s3_host`` usually switches the addresing style to virtual.
-        self.meta.events.unregister('before-sign.s3', fix_s3_host)
-
-    def _force_virtual_style_s3_addressing(self):
-        # If the virtual host addressing style is being forced,
-        # switch the default fix_s3_host handler for the more general
-        # switch_to_virtual_host_style handler that does not have opt out
-        # cases (other than throwing an error if the name is DNS incompatible)
-        self.meta.events.unregister('before-sign.s3', fix_s3_host)
-        self.meta.events.register(
-            'before-sign.s3', switch_to_virtual_host_style)
-
     @property
     def _service_model(self):
         return self.meta.service_model

botocore/handlers.py

@@ -818,7 +818,6 @@ def _replace_content(self, section):
     ('choose-signer.cognito-idp.SetUserSettings', disable_signing),
     ('choose-signer.cognito-idp.GetJWKS', disable_signing),
     ('choose-signer.cognito-idp.DeleteUserAttributes', disable_signing),
-    ('before-sign.s3', utils.fix_s3_host),
     ('before-parameter-build.s3.HeadObject', sse_md5),
     ('before-parameter-build.s3.GetObject', sse_md5),
     ('before-parameter-build.s3.PutObject', sse_md5),

botocore/utils.py

@@ -45,8 +45,8 @@
     'us-gov-west-1',
     'fips-us-gov-west-1',
 ]
-S3_ACCELERATE_ENDPOINT = 's3-accelerate.amazonaws.com'
 RETRYABLE_HTTP_ERRORS = (requests.Timeout, requests.ConnectionError)
+S3_ACCELERATE_WHITELIST = ['dualstack']
 
 
 class _RetriesExceededError(Exception):
@@ -775,7 +775,13 @@ def switch_host_s3_accelerate(request, operation_name, **kwargs):
     # before it gets changed to virtual. So we are not concerned with ensuring
     # that the bucket name is translated to the virtual style here and we
     # can hard code the Accelerate endpoint.
-    endpoint = 'https://' + S3_ACCELERATE_ENDPOINT
+    parts = urlsplit(request.url).netloc.split('.')
+    parts = [p for p in parts if p in S3_ACCELERATE_WHITELIST]
+    endpoint = 'https://s3-accelerate.'
+    if len(parts) > 0:
+        endpoint += '.'.join(parts) + '.'
+    endpoint += 'amazonaws.com'
+
     if operation_name in ['ListBuckets', 'CreateBucket', 'DeleteBucket']:
         return
     _switch_hosts(request, endpoint,  use_new_scheme=False)

tests/functional/test_s3.py

@@ -322,7 +322,6 @@ def test_generate_unauthed_post(self):
         self.assertEqual(parts, expected)
 
 
-
 def test_correct_url_used_for_s3():
     # Test that given various sets of config options and bucket names,
     # we construct the expect endpoint url.
@@ -401,7 +400,6 @@ def test_correct_url_used_for_s3():
         customer_provided_endpoint='https://foo.amazonaws.com/',
         expected_url='https://foo.amazonaws.com/bucket/key')
 
-
     # S3 accelerate
     use_accelerate = {'use_accelerate_endpoint': True}
     yield t.case(
@@ -413,6 +411,7 @@ def test_correct_url_used_for_s3():
         region='us-west-2', bucket='bucket', key='key',
         s3_config=use_accelerate,
         expected_url='https://bucket.s3-accelerate.amazonaws.com/key')
+    # Provided endpoints still get recognized as accelerate endpoints.
     yield t.case(
         region='us-east-1', bucket='bucket', key='key',
         customer_provided_endpoint='https://s3-accelerate.amazonaws.com',
@@ -426,6 +425,21 @@ def test_correct_url_used_for_s3():
         s3_config=use_accelerate, is_secure=False,
         # Note we're using http://  because is_secure=False.
         expected_url='http://bucket.s3-accelerate.amazonaws.com/key')
+    yield t.case(
+        region='us-east-1', bucket='bucket', key='key',
+        # s3-accelerate must be the first part of the url.
+        customer_provided_endpoint='https://foo.s3-accelerate.amazonaws.com',
+        expected_url='https://foo.s3-accelerate.amazonaws.com/bucket/key')
+    yield t.case(
+        region='us-east-1', bucket='bucket', key='key',
+        # The endpoint must be an Amazon endpoint.
+        customer_provided_endpoint='https://s3-accelerate.notamazon.com',
+        expected_url='https://s3-accelerate.notamazon.com/bucket/key')
+    yield t.case(
+        region='us-east-1', bucket='bucket', key='key',
+        # Extra components must be whitelisted.
+        customer_provided_endpoint='https://s3-accelerate.foo.amazonaws.com',
+        expected_url='https://s3-accelerate.foo.amazonaws.com/bucket/key')
     # Use virtual even if path is specified for s3 accelerate because
     # path style will not work with S3 accelerate.
     yield t.case(
@@ -445,19 +459,22 @@ def test_correct_url_used_for_s3():
         region='us-west-2', bucket='bucket', key='key',
         s3_config=use_dualstack,
         # Still default to virtual hosted when possible.
-        expected_url='https://bucket.s3.dualstack.us-west-2.amazonaws.com/key')
+        expected_url=(
+            'https://bucket.s3.dualstack.us-west-2.amazonaws.com/key'))
     # Non DNS compatible buckets use path style for dual stack.
     yield t.case(
         region='us-west-2', bucket='bucket.dot', key='key',
         s3_config=use_dualstack,
         # Still default to virtual hosted when possible.
-        expected_url='https://s3.dualstack.us-west-2.amazonaws.com/bucket.dot/key')
+        expected_url=(
+            'https://s3.dualstack.us-west-2.amazonaws.com/bucket.dot/key'))
     # Supports is_secure (use_ssl=False in create_client()).
     yield t.case(
         region='us-west-2', bucket='bucket.dot', key='key', is_secure=False,
         s3_config=use_dualstack,
         # Still default to virtual hosted when possible.
-        expected_url='http://s3.dualstack.us-west-2.amazonaws.com/bucket.dot/key')
+        expected_url=(
+            'http://s3.dualstack.us-west-2.amazonaws.com/bucket.dot/key'))
 
     # Is path style is requested, we should use it, even if the bucket is
     # DNS compatible.
@@ -471,6 +488,73 @@ def test_correct_url_used_for_s3():
         # Still default to virtual hosted when possible.
         expected_url='https://s3.dualstack.us-west-2.amazonaws.com/bucket/key')
 
+    # Accelerate + dual stack
+    use_accelerate_dualstack = {
+        'use_accelerate_endpoint': True,
+        'use_dualstack_endpoint': True,
+    }
+    yield t.case(
+        region='us-east-1', bucket='bucket', key='key',
+        s3_config=use_accelerate_dualstack,
+        expected_url=(
+            'https://bucket.s3-accelerate.dualstack.amazonaws.com/key'))
+    yield t.case(
+        # Region is ignored with S3 accelerate.
+        region='us-west-2', bucket='bucket', key='key',
+        s3_config=use_accelerate_dualstack,
+        expected_url=(
+            'https://bucket.s3-accelerate.dualstack.amazonaws.com/key'))
+    # Only s3-accelerate overrides a customer endpoint.
+    yield t.case(
+        region='us-east-1', bucket='bucket', key='key',
+        s3_config=use_dualstack,
+        customer_provided_endpoint='https://s3-accelerate.amazonaws.com',
+        expected_url=(
+            'https://bucket.s3-accelerate.amazonaws.com/key'))
+    yield t.case(
+        region='us-east-1', bucket='bucket', key='key',
+        # Dualstack is whitelisted.
+        customer_provided_endpoint=(
+            'https://s3-accelerate.dualstack.amazonaws.com'),
+        expected_url=(
+            'https://bucket.s3-accelerate.dualstack.amazonaws.com/key'))
+    yield t.case(
+        region='us-east-1', bucket='bucket', key='key',
+        # Even whitelisted parts cannot be duplicated.
+        customer_provided_endpoint=(
+            'https://s3-accelerate.dualstack.dualstack.amazonaws.com'),
+        expected_url=(
+            'https://s3-accelerate.dualstack.dualstack'
+            '.amazonaws.com/bucket/key'))
+    yield t.case(
+        region='us-east-1', bucket='bucket', key='key',
+        # More than two extra parts is not allowed.
+        customer_provided_endpoint=(
+            'https://s3-accelerate.dualstack.dualstack.dualstack'
+            '.amazonaws.com'),
+        expected_url=(
+            'https://s3-accelerate.dualstack.dualstack.dualstack.amazonaws.com'
+            '/bucket/key'))
+    yield t.case(
+        region='us-east-1', bucket='bucket', key='key',
+        # Extra components must be whitelisted.
+        customer_provided_endpoint='https://s3-accelerate.foo.amazonaws.com',
+        expected_url='https://s3-accelerate.foo.amazonaws.com/bucket/key')
+    yield t.case(
+        region='us-east-1', bucket='bucket', key='key',
+        s3_config=use_accelerate_dualstack, is_secure=False,
+        # Note we're using http://  because is_secure=False.
+        expected_url=(
+            'http://bucket.s3-accelerate.dualstack.amazonaws.com/key'))
+    # Use virtual even if path is specified for s3 accelerate because
+    # path style will not work with S3 accelerate.
+    use_accelerate_dualstack['addressing_style'] = 'path'
+    yield t.case(
+        region='us-east-1', bucket='bucket', key='key',
+        s3_config=use_accelerate_dualstack,
+        expected_url=(
+            'https://bucket.s3-accelerate.dualstack.amazonaws.com/key'))
+
 
 class S3AddressingCases(object):
     def __init__(self, verify_function):