Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix s3 fips endpoint #265

Merged
merged 2 commits into from
Mar 28, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion botocore/data/aws/s3/2006-03-01.json
Original file line number Diff line number Diff line change
Expand Up @@ -5298,7 +5298,7 @@
"us-west-1": "https://s3-us-west-1.amazonaws.com/",
"eu-west-1": "https://s3-eu-west-1.amazonaws.com/",
"us-gov-west-1": "https://s3-us-gov-west-1.amazonaws.com/",
"fips-gov-west-1": "https://s3-fips-us-gov-west-1.amazonaws.com/",
"fips-us-gov-west-1": "https://s3-fips-us-gov-west-1.amazonaws.com/",
"cn-north-1": "https://s3.cn-north-1.amazonaws.com.cn"
},
"protocols": [
Expand Down
2 changes: 1 addition & 1 deletion botocore/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
LABEL_RE = re.compile('[a-z0-9][a-z0-9\-]*[a-z0-9]')
RESTRICTED_REGIONS = [
'us-gov-west-1',
'fips-gov-west-1',
'fips-us-gov-west-1',
]


Expand Down
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
2 changes: 1 addition & 1 deletion services/s3.extra.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"us-west-1": "https://s3-us-west-1.amazonaws.com/",
"eu-west-1": "https://s3-eu-west-1.amazonaws.com/",
"us-gov-west-1": "https://s3-us-gov-west-1.amazonaws.com/",
"fips-gov-west-1": "https://s3-fips-us-gov-west-1.amazonaws.com/",
"fips-us-gov-west-1": "https://s3-fips-us-gov-west-1.amazonaws.com/",
"cn-north-1": "https://s3.cn-north-1.amazonaws.com.cn"
},
"protocols": [
Expand Down
50 changes: 43 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 @@ -68,6 +69,16 @@ def test_list_objects_in_restricted_regions(self):
self.assertEqual(prepared_request.url,
'https://s3-us-gov-west-1.amazonaws.com/safename')

def test_list_objects_in_fips(self):
self.endpoint = self.s3.get_endpoint('fips-us-gov-west-1')
op = self.s3.get_operation('ListObjects')
params = op.build_parameters(bucket='safename')
prepared_request = self.get_prepared_request(op, params)
# Note how we keep the region specific endpoint here.
self.assertEqual(
prepared_request.url,
'https://s3-fips-us-gov-west-1.amazonaws.com/safename')

def test_list_objects_non_dns_name_non_classic(self):
self.endpoint = self.s3.get_endpoint('us-west-2')
op = self.s3.get_operation('ListObjects')
Expand Down Expand Up @@ -146,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')