From c88c779413b36e333224ef68c8489c6eab67d865 Mon Sep 17 00:00:00 2001 From: Mathieu Guillaume Date: Fri, 8 Nov 2013 18:19:00 +0100 Subject: [PATCH 1/4] Add back support for --endpoint-url option to s3 --- awscli/customizations/s3/s3.py | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/awscli/customizations/s3/s3.py b/awscli/customizations/s3/s3.py index c1d0ff9adf9a..39a99f8d3bc4 100644 --- a/awscli/customizations/s3/s3.py +++ b/awscli/customizations/s3/s3.py @@ -279,6 +279,7 @@ def _do_command(self, parsed_args, parsed_globals): params = self._build_call_parameters(parsed_args, {}) cmd_params = CommandParameters(self._session, self._name, params) cmd_params.check_region(parsed_globals) + cmd_params.check_endpoint_url(parsed_globals) cmd_params.add_paths(parsed_args.paths) cmd_params.check_force(parsed_globals) cmd = CommandArchitecture(self._session, self._name, @@ -479,7 +480,8 @@ def __init__(self, session, cmd, parameters): self.parameters = parameters self.instructions = [] self._service = self.session.get_service('s3') - self._endpoint = self._service.get_endpoint(self.parameters['region']) + self._endpoint = self._service.get_endpoint(region_name=self.parameters['region'], + endpoint_url=self.parameters['endpoint_url']) def create_instructions(self): """ @@ -772,15 +774,31 @@ def check_region(self, parsed_globals): parsed_region = None if 'region' in parsed_globals: parsed_region = getattr(parsed_globals, 'region') - if not region and not parsed_region: + if 'endpoint_url' in parsed_globals: + parsed_endpoint_url = getattr(parsed_globals, 'endpoint_url') + else: + parsed_endpoint_url = None + if not region and not parsed_region and parsed_endpoint_url is None: raise Exception("A region must be specified --region or " "specifying the region\nin a configuration " - "file or as an environment variable\n") + "file or as an environment variable.\n" + "Alternately, an endpoint can be specified " + "with --endpoint-url") if parsed_region: self.parameters['region'] = parsed_region - else: + elif region: self.parameters['region'] = region + else: + self.parameters['region'] = None + def check_endpoint_url(self, parsed_globals): + """ + Adds endpoint_url to the parameters. + """ + if 'endpoint_url' in parsed_globals: + self.parameters['endpoint_url'] = getattr(parsed_globals, 'endpoint_url') + else: + self.parameters['endpoint_url'] = None # This is a dictionary useful for automatically adding the different commands, # the amount of arguments it takes, and the optional parameters that can appear From ab2e5e587143865042999684b88082bd3db17b3f Mon Sep 17 00:00:00 2001 From: Mathieu Guillaume Date: Fri, 8 Nov 2013 18:32:06 +0100 Subject: [PATCH 2/4] Update S3 unit tests for endpoint_url param --- awscli/customizations/s3/s3.py | 2 +- tests/unit/customizations/s3/fake_session.py | 4 +-- tests/unit/customizations/s3/test_s3.py | 28 ++++++++++++-------- 3 files changed, 20 insertions(+), 14 deletions(-) diff --git a/awscli/customizations/s3/s3.py b/awscli/customizations/s3/s3.py index 39a99f8d3bc4..75829989a416 100644 --- a/awscli/customizations/s3/s3.py +++ b/awscli/customizations/s3/s3.py @@ -791,7 +791,7 @@ def check_region(self, parsed_globals): else: self.parameters['region'] = None - def check_endpoint_url(self, parsed_globals): + def check_endpoint_url(self, parsed_globals): """ Adds endpoint_url to the parameters. """ diff --git a/tests/unit/customizations/s3/fake_session.py b/tests/unit/customizations/s3/fake_session.py index f89f5d281553..6fe6691f854a 100644 --- a/tests/unit/customizations/s3/fake_session.py +++ b/tests/unit/customizations/s3/fake_session.py @@ -77,9 +77,9 @@ class FakeService(object): def __init__(self, session): self.session = session - def get_endpoint(self, region): + def get_endpoint(self, region_name, endpoint_url=None): endpoint = Mock() - endpoint.region_name = region + endpoint.region_name = region_name return endpoint def get_operation(self, name): diff --git a/tests/unit/customizations/s3/test_s3.py b/tests/unit/customizations/s3/test_s3.py index decf10c931b9..5f29fcbe034a 100644 --- a/tests/unit/customizations/s3/test_s3.py +++ b/tests/unit/customizations/s3/test_s3.py @@ -171,9 +171,9 @@ def test_create_instructions(self): 'mb': ['s3_handler'], 'rb': ['s3_handler']} - params = {'filters': True, 'region': 'us-east-1'} + params = {'filters': True, 'region': 'us-east-1', 'endpoint_url': None} for cmd in cmds: - cmd_arc = CommandArchitecture(self.session, cmd, {'region': 'us-east-1'}) + cmd_arc = CommandArchitecture(self.session, cmd, {'region': 'us-east-1', 'endpoint_url': None}) cmd_arc.create_instructions() self.assertEqual(cmd_arc.instructions, instructions[cmd]) @@ -193,7 +193,8 @@ def test_run_cp_put(self): filters = [['--include', '*']] params = {'dir_op': False, 'dryrun': True, 'quiet': False, 'src': local_file, 'dest': s3_file, 'filters': filters, - 'paths_type': 'locals3', 'region': 'us-east-1'} + 'paths_type': 'locals3', 'region': 'us-east-1', + 'endpoint_url': None} cmd_arc = CommandArchitecture(self.session, 'cp', params) cmd_arc.create_instructions() cmd_arc.run() @@ -210,7 +211,8 @@ def test_run_cp_get(self): filters = [['--include', '*']] params = {'dir_op': False, 'dryrun': True, 'quiet': False, 'src': s3_file, 'dest': local_file, 'filters': filters, - 'paths_type': 's3local', 'region': 'us-east-1'} + 'paths_type': 's3local', 'region': 'us-east-1', + 'endpoint_url': None} cmd_arc = CommandArchitecture(self.session, 'cp', params) cmd_arc.create_instructions() cmd_arc.run() @@ -225,7 +227,8 @@ def test_run_cp_copy(self): filters = [['--include', '*']] params = {'dir_op': False, 'dryrun': True, 'quiet': False, 'src': s3_file, 'dest': s3_file, 'filters': filters, - 'paths_type': 's3s3', 'region': 'us-east-1'} + 'paths_type': 's3s3', 'region': 'us-east-1', + 'endpoint_url': None} cmd_arc = CommandArchitecture(self.session, 'cp', params) cmd_arc.create_instructions() cmd_arc.run() @@ -240,7 +243,8 @@ def test_run_mv(self): filters = [['--include', '*']] params = {'dir_op': False, 'dryrun': True, 'quiet': False, 'src': s3_file, 'dest': s3_file, 'filters': filters, - 'paths_type': 's3s3', 'region': 'us-east-1'} + 'paths_type': 's3s3', 'region': 'us-east-1', + 'endpoint_url': None} cmd_arc = CommandArchitecture(self.session, 'mv', params) cmd_arc.create_instructions() cmd_arc.run() @@ -255,7 +259,8 @@ def test_run_remove(self): filters = [['--include', '*']] params = {'dir_op': False, 'dryrun': True, 'quiet': False, 'src': s3_file, 'dest': s3_file, 'filters': filters, - 'paths_type': 's3', 'region': 'us-east-1'} + 'paths_type': 's3', 'region': 'us-east-1', + 'endpoint_url': None} cmd_arc = CommandArchitecture(self.session, 'rm', params) cmd_arc.create_instructions() cmd_arc.run() @@ -274,7 +279,8 @@ def test_run_sync(self): filters = [['--include', '*']] params = {'dir_op': True, 'dryrun': True, 'quiet': False, 'src': local_dir, 'dest': s3_prefix, 'filters': filters, - 'paths_type': 'locals3', 'region': 'us-east-1'} + 'paths_type': 'locals3', 'region': 'us-east-1', + 'endpoint_url': None} cmd_arc = CommandArchitecture(self.session, 'sync', params) cmd_arc.create_instructions() cmd_arc.run() @@ -288,7 +294,7 @@ def test_run_mb(self): s3_prefix = 's3://' + self.bucket + '/' params = {'dir_op': True, 'dryrun': True, 'quiet': False, 'src': s3_prefix, 'dest': s3_prefix, 'paths_type': 's3', - 'region': 'us-east-1'} + 'region': 'us-east-1', 'endpoint_url': None} cmd_arc = CommandArchitecture(self.session, 'mb', params) cmd_arc.create_instructions() cmd_arc.run() @@ -302,7 +308,7 @@ def test_run_rb(self): s3_prefix = 's3://' + self.bucket + '/' params = {'dir_op': True, 'dryrun': True, 'quiet': False, 'src': s3_prefix, 'dest': s3_prefix, 'paths_type': 's3', - 'region': 'us-east-1'} + 'region': 'us-east-1', 'endpoint_url': None} cmd_arc = CommandArchitecture(self.session, 'rb', params) cmd_arc.create_instructions() rc = cmd_arc.run() @@ -317,7 +323,7 @@ def test_run_rb_nonzero_rc(self): s3_prefix = 's3://' + self.bucket + '/' params = {'dir_op': True, 'dryrun': False, 'quiet': False, 'src': s3_prefix, 'dest': s3_prefix, 'paths_type': 's3', - 'region': 'us-east-1'} + 'region': 'us-east-1', 'endpoint_url': None} cmd_arc = CommandArchitecture(self.session, 'rb', params) cmd_arc.create_instructions() rc = cmd_arc.run() From 85a3127d0ac59206b9ae956100416e3875938be8 Mon Sep 17 00:00:00 2001 From: Mathieu Guillaume Date: Fri, 8 Nov 2013 18:51:32 +0100 Subject: [PATCH 3/4] Add unit test to ensure endpoint-url is passed to botocore for s3 --- tests/unit/test_clidriver.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/unit/test_clidriver.py b/tests/unit/test_clidriver.py index 0e0463ef730b..756855fa968a 100644 --- a/tests/unit/test_clidriver.py +++ b/tests/unit/test_clidriver.py @@ -330,6 +330,17 @@ def test_aws_with_region(self): endpoint.assert_called_with(region_name='us-east-1', endpoint_url=None) + def test_s3_with_region_and_endpoint_url(self): + with mock.patch('botocore.service.Service.get_endpoint') as endpoint: + http_response = models.Response() + http_response.status_code = 200 + endpoint.return_value.make_request.return_value = ( + http_response, {}) + self.assert_params_for_cmd( + 's3 ls s3://test --region us-east-1 --endpoint-url https://foobar.com/', + expected_rc=0) + endpoint.assert_called_with(region_name='us-east-1', + endpoint_url='https://foobar.com/') def inject_new_param(self, argument_table, **kwargs): argument = CustomArgument('unknown-arg', {}) From 6629b2980f096d070733190696d93e2b5c2bd3b9 Mon Sep 17 00:00:00 2001 From: James Saryerwinnie Date: Mon, 18 Nov 2013 14:04:18 -0800 Subject: [PATCH 4/4] Update ls to support endpoint-url --- awscli/customizations/s3/s3.py | 13 +++++++++---- tests/unit/test_clidriver.py | 2 +- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/awscli/customizations/s3/s3.py b/awscli/customizations/s3/s3.py index 75829989a416..905c83049f13 100644 --- a/awscli/customizations/s3/s3.py +++ b/awscli/customizations/s3/s3.py @@ -283,7 +283,7 @@ def _do_command(self, parsed_args, parsed_globals): cmd_params.add_paths(parsed_args.paths) cmd_params.check_force(parsed_globals) cmd = CommandArchitecture(self._session, self._name, - cmd_params.parameters) + cmd_params.parameters) cmd.create_instructions() return cmd.run() @@ -342,12 +342,16 @@ def _create_operation_parser(self, parameter_table): parser.add_argument("paths", **self.options) return parser + def _get_endpoint(self, service, parsed_globals): + return service.get_endpoint(region_name=parsed_globals.region, + endpoint_url=parsed_globals.endpoint_url) + class ListCommand(S3SubCommand): def _do_command(self, parsed_args, parsed_globals): bucket, key = find_bucket_key(parsed_args.paths[0][5:]) self.service = self._session.get_service('s3') - self.endpoint = self.service.get_endpoint(parsed_globals.region) + self.endpoint = self._get_endpoint(self.service, parsed_globals) if not bucket: self._list_all_buckets() else: @@ -480,8 +484,9 @@ def __init__(self, session, cmd, parameters): self.parameters = parameters self.instructions = [] self._service = self.session.get_service('s3') - self._endpoint = self._service.get_endpoint(region_name=self.parameters['region'], - endpoint_url=self.parameters['endpoint_url']) + self._endpoint = self._service.get_endpoint( + region_name=self.parameters['region'], + endpoint_url=self.parameters['endpoint_url']) def create_instructions(self): """ diff --git a/tests/unit/test_clidriver.py b/tests/unit/test_clidriver.py index 756855fa968a..7270134b2fbd 100644 --- a/tests/unit/test_clidriver.py +++ b/tests/unit/test_clidriver.py @@ -335,7 +335,7 @@ def test_s3_with_region_and_endpoint_url(self): http_response = models.Response() http_response.status_code = 200 endpoint.return_value.make_request.return_value = ( - http_response, {}) + http_response, {'CommonPrefixes': [], 'Contents': []}) self.assert_params_for_cmd( 's3 ls s3://test --region us-east-1 --endpoint-url https://foobar.com/', expected_rc=0)