Skip to content

Commit

Permalink
Add test for endpoint global logic
Browse files Browse the repository at this point in the history
This didn't make sense to me at first, but for a number of reasons
(noted in the comments) we can't change this behavior just yet.
Otherwise this would be a breaking change to the CLI.  I think there's
probably a better way to do this, but for now leaving things as is.  I
did add a test that documents this behavior though, so hopefully
others don't hit this code and wonder what's going on.
  • Loading branch information
jamesls committed Mar 27, 2014
1 parent 2b58065 commit f84eaaa
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 7 deletions.
6 changes: 6 additions & 0 deletions botocore/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,12 @@ def get_endpoint(self, region_name=None, is_secure=True,
# endpoint, we can just use the global_endpoint (which is a
# string of the hostname of the global endpoint) to construct
# the full endpoint_url.
# We have to be careful though. The "region_name" should have
# been previously validated, otherwise it's possible
# that we may fail silently if the user provided a region
# we don't know about. For example,
# s3.get_endpoint('bad region') would return the global
# s3 endpoint, which is probably not what we want.
endpoint_url = self._build_endpoint_url(self.global_endpoint,
is_secure)
region_name = 'us-east-1'
Expand Down
40 changes: 33 additions & 7 deletions tests/unit/test_s3_addressing.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
from mock import patch, Mock

import botocore.session
from botocore.exceptions import ServiceNotInRegionError


class TestS3Addressing(BaseSessionTest):
Expand Down Expand Up @@ -156,20 +157,45 @@ def test_get_object_non_dns_name_classic(self):
'https://s3.amazonaws.com/AnInvalidName/mykeyname')

def test_get_object_ip_address_name_non_classic(self):
self.endpoint = self.s3.get_endpoint('us-west-s')
self.endpoint = self.s3.get_endpoint('us-west-2')
op = self.s3.get_operation('GetObject')
params = op.build_parameters(bucket='192.168.5.4',
key='mykeyname')
prepared_request = self.get_prepared_request(op, params)
self.assertEqual(prepared_request.url,
'https://s3.amazonaws.com/192.168.5.4/mykeyname')

self.assertEqual(
prepared_request.url,
'https://s3-us-west-2.amazonaws.com/192.168.5.4/mykeyname')

def test_get_object_almost_an_ip_address_name_non_classic(self):
self.endpoint = self.s3.get_endpoint('us-west-s')
self.endpoint = self.s3.get_endpoint('us-west-2')
op = self.s3.get_operation('GetObject')
params = op.build_parameters(bucket='192.168.5.256',
key='mykeyname')
prepared_request = self.get_prepared_request(op, params)
self.assertEqual(prepared_request.url,
'https://s3.amazonaws.com/192.168.5.256/mykeyname')
self.assertEqual(
prepared_request.url,
'https://s3-us-west-2.amazonaws.com/192.168.5.256/mykeyname')

def test_non_existent_region(self):
# XXX: This is something I think we need to address in the future
# but it at least needs to be documented/tested so we know
# the current behavior. If I ask for a region that does not
# exist on a global endpoint, such as:
endpoint = self.s3.get_endpoint('REGION DOES NOT EXIST')
# I get the global endpoint.
self.assertEqual(endpoint.region_name, 'us-east-1')
# Why not fixed this? Well backwards compatability for one thing.
# The other reason is because it was intended to accomodate this
# use case. Let's say I have us-west-2 set as my default region,
# possibly through an env var or config variable. Well, by default,
# we'd make a call like:
iam_endpoint = self.session.get_service('iam').get_endpoint('us-west-2')
# Instead of giving the user an error, we should instead give
# them the global endpoint.
self.assertEqual(iam_endpoint.region_name, 'us-east-1')
# But if they request an endpoint that we *do* know about, we use
# that specific endpoint.
self.assertEqual(
self.session.get_service('iam').get_endpoint(
'us-gov-west-1').region_name,
'us-gov-west-1')

0 comments on commit f84eaaa

Please sign in to comment.