From 8b4afd269e4064100a7bb3bb2f2b94995a2c5347 Mon Sep 17 00:00:00 2001 From: Jonas Neubert Date: Wed, 26 Apr 2023 09:35:02 -0600 Subject: [PATCH 1/7] emit provide-client-params.* and before-parameter-build.* before endpoint resolution --- botocore/client.py | 9 +++++---- botocore/signers.py | 12 ++++++++++-- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/botocore/client.py b/botocore/client.py index da92f39987..7180779414 100644 --- a/botocore/client.py +++ b/botocore/client.py @@ -819,7 +819,6 @@ def _resolve_signature_version(self, service_name, resolved): class BaseClient: - # This is actually reassigned with the py->op_name mapping # when the client creator creates the subclass. This value is used # because calls such as client.get_paginator('list_objects') use the @@ -913,6 +912,11 @@ def _make_api_call(self, operation_name, api_params): 'has_streaming_input': operation_model.has_streaming_input, 'auth_type': operation_model.auth_type, } + api_params = self._emit_api_params( + api_params=api_params, + operation_model=operation_model, + context=request_context, + ) endpoint_url, additional_headers = self._resolve_endpoint_ruleset( operation_model, api_params, request_context ) @@ -984,9 +988,6 @@ def _convert_to_request_dict( headers=None, set_user_agent_header=True, ): - api_params = self._emit_api_params( - api_params, operation_model, context - ) request_dict = self._serializer.serialize_to_request( api_params, operation_model ) diff --git a/botocore/signers.py b/botocore/signers.py index 42965b41e4..8ff0fdd1b2 100644 --- a/botocore/signers.py +++ b/botocore/signers.py @@ -663,7 +663,11 @@ def generate_presigned_url( context, ignore_signing_region=(not bucket_is_arn), ) - + params = self._emit_api_params( + api_params=params, + operation_model=operation_model, + context=context, + ) request_dict = self._convert_to_request_dict( api_params=params, operation_model=operation_model, @@ -786,7 +790,11 @@ def generate_presigned_post( context, ignore_signing_region=(not bucket_is_arn), ) - + params = self._emit_api_params( + api_params=params, + operation_model=operation_model, + context=context, + ) request_dict = self._convert_to_request_dict( api_params=params, operation_model=operation_model, From dd415601d5df5ae273e1a261458e68720dd0b78c Mon Sep 17 00:00:00 2001 From: Jonas Neubert Date: Wed, 26 Apr 2023 16:46:46 -0600 Subject: [PATCH 2/7] test coverage for dynamic ctx params set in provide-client-param event handler --- tests/functional/test_context_params.py | 41 ++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/tests/functional/test_context_params.py b/tests/functional/test_context_params.py index f61e6a18fc..d72b65b265 100644 --- a/tests/functional/test_context_params.py +++ b/tests/functional/test_context_params.py @@ -343,7 +343,7 @@ def test_client_context_param_sent_to_endpoint_resolver( ), ) - # Stub client to prevent a request from getting sent and asceertain that + # Stub client to prevent a request from getting sent and ascertain that # only a single request would get sent. Wrap the EndpointProvider's # resolve_endpoint method for inspecting the arguments it gets called with. with ClientHTTPStubber(client, strict=True) as http_stubber: @@ -488,3 +488,42 @@ def test_dynamic_context_param_sent_to_endpoint_resolver( ) else: mock_resolve_endpoint.assert_called_once_with(Region='us-east-1') + + +def test_dynamic_context_param_from_event_handler_sent_to_endpoint_resolver( + monkeypatch, + patched_session, +): + # patch loader to return fake service model and fake endpoint ruleset + patch_load_service_model( + patched_session, + monkeypatch, + FAKE_MODEL_WITH_DYNAMIC_CONTEXT_PARAM, + FAKE_RULESET_WITH_DYNAMIC_CONTEXT_PARAM, + ) + + # event handler for provide-client-params that modifies the value of the + # MockOpParam parameter + def change_param(params, **kwargs): + params['MockOpParam'] = 'mock-op-param-value-2' + + client = patched_session.create_client( + 'otherservice', region_name='us-east-1' + ) + client.meta.events.register_last( + 'provide-client-params.other-service.*', change_param + ) + + with ClientHTTPStubber(client, strict=True) as http_stubber: + http_stubber.add_response(status=200) + with mock.patch.object( + client._ruleset_resolver._provider, + 'resolve_endpoint', + wraps=client._ruleset_resolver._provider.resolve_endpoint, + ) as mock_resolve_endpoint: + client.mock_operation(MockOpParam='mock-op-param-value-1') + + mock_resolve_endpoint.assert_called_once_with( + Region='us-east-1', + FooDynamicContextParamName='mock-op-param-value-2', + ) From bd4452461678a26b57cb8ccf1db9802f1fda8109 Mon Sep 17 00:00:00 2001 From: Jonas Neubert Date: Mon, 8 May 2023 16:45:32 -0600 Subject: [PATCH 3/7] consolidate repeated code in _convert_to_request_dict --- botocore/client.py | 26 +++++++++++++++----------- botocore/signers.py | 28 ++-------------------------- 2 files changed, 17 insertions(+), 37 deletions(-) diff --git a/botocore/client.py b/botocore/client.py index 7180779414..390f06c02a 100644 --- a/botocore/client.py +++ b/botocore/client.py @@ -912,20 +912,10 @@ def _make_api_call(self, operation_name, api_params): 'has_streaming_input': operation_model.has_streaming_input, 'auth_type': operation_model.auth_type, } - api_params = self._emit_api_params( - api_params=api_params, - operation_model=operation_model, - context=request_context, - ) - endpoint_url, additional_headers = self._resolve_endpoint_ruleset( - operation_model, api_params, request_context - ) request_dict = self._convert_to_request_dict( api_params=api_params, operation_model=operation_model, - endpoint_url=endpoint_url, context=request_context, - headers=additional_headers, ) resolve_checksum_context(request_dict, operation_model, api_params) @@ -983,11 +973,23 @@ def _convert_to_request_dict( self, api_params, operation_model, - endpoint_url, + endpoint_url=None, context=None, headers=None, set_user_agent_header=True, + ignore_signing_region=False, ): + api_params = self._emit_api_params( + api_params=api_params, + operation_model=operation_model, + context=context, + ) + endpoint_url, additional_headers = self._resolve_endpoint_ruleset( + operation_model=operation_model, + params=api_params, + request_context=context, + ignore_signing_region=ignore_signing_region, + ) request_dict = self._serializer.serialize_to_request( api_params, operation_model ) @@ -995,6 +997,8 @@ def _convert_to_request_dict( request_dict.pop('host_prefix', None) if headers is not None: request_dict['headers'].update(headers) + if additional_headers is not None: + request_dict['headers'].update(additional_headers) if set_user_agent_header: user_agent = self._client_config.user_agent else: diff --git a/botocore/signers.py b/botocore/signers.py index 8ff0fdd1b2..3d82158e56 100644 --- a/botocore/signers.py +++ b/botocore/signers.py @@ -657,24 +657,12 @@ def generate_presigned_url( operation_model = self.meta.service_model.operation_model(operation_name) bucket_is_arn = ArnParser.is_arn(params.get('Bucket', '')) - endpoint_url, additional_headers = self._resolve_endpoint_ruleset( - operation_model, - params, - context, - ignore_signing_region=(not bucket_is_arn), - ) - params = self._emit_api_params( - api_params=params, - operation_model=operation_model, - context=context, - ) request_dict = self._convert_to_request_dict( api_params=params, operation_model=operation_model, - endpoint_url=endpoint_url, context=context, - headers=additional_headers, set_user_agent_header=False, + ignore_signing_region=(not bucket_is_arn), ) # Switch out the http method if user specified it. @@ -784,24 +772,12 @@ def generate_presigned_post( operation_model = self.meta.service_model.operation_model('CreateBucket') params = {'Bucket': bucket} bucket_is_arn = ArnParser.is_arn(params.get('Bucket', '')) - endpoint_url, additional_headers = self._resolve_endpoint_ruleset( - operation_model, - params, - context, - ignore_signing_region=(not bucket_is_arn), - ) - params = self._emit_api_params( - api_params=params, - operation_model=operation_model, - context=context, - ) request_dict = self._convert_to_request_dict( api_params=params, operation_model=operation_model, - endpoint_url=endpoint_url, context=context, - headers=additional_headers, set_user_agent_header=False, + ignore_signing_region=(not bucket_is_arn), ) # Append that the bucket name to the list of conditions. From 763d1f7d5957f8f9a49950ee54ec802d79be8070 Mon Sep 17 00:00:00 2001 From: Jonas Neubert Date: Mon, 8 May 2023 16:46:16 -0600 Subject: [PATCH 4/7] test coverage for provide-client-param event handler + presigned URLs --- tests/unit/test_signers.py | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/tests/unit/test_signers.py b/tests/unit/test_signers.py index 4b0baefe2e..5b05c7fea8 100644 --- a/tests/unit/test_signers.py +++ b/tests/unit/test_signers.py @@ -991,6 +991,37 @@ def test_generate_presign_url_emits_is_presign_in_context(self): 'the following kwargs emitted: %s' % kwargs, ) + def test_context_param_from_event_handler_sent_to_endpoint_resolver(self): + def change_bucket_param(params, **kwargs): + params['Bucket'] = 'mybucket-bar' + + self.client.meta.events.register_last( + 'provide-client-params.s3.*', change_bucket_param + ) + + self.client.generate_presigned_url( + 'get_object', Params={'Bucket': 'mybucket-foo', 'Key': self.key} + ) + + ref_request_dict = { + 'body': b'', + # If the bucket name set in the provide-client-params event handler + # was correctly passed to the endpoint provider as a dynamic context + # parameter, it will appear in the URL and the auth_path: + 'url': 'https://mybucket-bar.s3.amazonaws.com/mykey', + 'headers': {}, + 'auth_path': '/mybucket-bar/mykey', + 'query_string': {}, + 'url_path': '/mykey', + 'method': 'GET', + 'context': mock.ANY, + } + self.generate_url_mock.assert_called_with( + request_dict=ref_request_dict, + expires_in=3600, + operation_name='GetObject', + ) + class TestGeneratePresignedPost(unittest.TestCase): def setUp(self): From e4c89a518b89ccf36ebef86c0a59b4fb21573083 Mon Sep 17 00:00:00 2001 From: Jonas Neubert Date: Tue, 9 May 2023 11:19:34 -0600 Subject: [PATCH 5/7] changelog --- .changes/next-release/bugfix-endpoints-38658.json | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changes/next-release/bugfix-endpoints-38658.json diff --git a/.changes/next-release/bugfix-endpoints-38658.json b/.changes/next-release/bugfix-endpoints-38658.json new file mode 100644 index 0000000000..1bdfba2953 --- /dev/null +++ b/.changes/next-release/bugfix-endpoints-38658.json @@ -0,0 +1,5 @@ +{ + "type": "bugfix", + "category": "endpoints", + "description": "Include params set in provide-client-param event handlers in dynamic context params for endpoint resolution." +} From 878cd844568c6404d24b14e148bc348abc9e0e59 Mon Sep 17 00:00:00 2001 From: Jonas Neubert Date: Tue, 9 May 2023 13:22:16 -0600 Subject: [PATCH 6/7] Revert "consolidate repeated code in _convert_to_request_dict" This reverts commit bd4452461678a26b57cb8ccf1db9802f1fda8109. --- botocore/client.py | 26 +++++++++++--------------- botocore/signers.py | 29 ++++++++++++++++++++++++++--- 2 files changed, 37 insertions(+), 18 deletions(-) diff --git a/botocore/client.py b/botocore/client.py index 390f06c02a..7180779414 100644 --- a/botocore/client.py +++ b/botocore/client.py @@ -912,10 +912,20 @@ def _make_api_call(self, operation_name, api_params): 'has_streaming_input': operation_model.has_streaming_input, 'auth_type': operation_model.auth_type, } + api_params = self._emit_api_params( + api_params=api_params, + operation_model=operation_model, + context=request_context, + ) + endpoint_url, additional_headers = self._resolve_endpoint_ruleset( + operation_model, api_params, request_context + ) request_dict = self._convert_to_request_dict( api_params=api_params, operation_model=operation_model, + endpoint_url=endpoint_url, context=request_context, + headers=additional_headers, ) resolve_checksum_context(request_dict, operation_model, api_params) @@ -973,23 +983,11 @@ def _convert_to_request_dict( self, api_params, operation_model, - endpoint_url=None, + endpoint_url, context=None, headers=None, set_user_agent_header=True, - ignore_signing_region=False, ): - api_params = self._emit_api_params( - api_params=api_params, - operation_model=operation_model, - context=context, - ) - endpoint_url, additional_headers = self._resolve_endpoint_ruleset( - operation_model=operation_model, - params=api_params, - request_context=context, - ignore_signing_region=ignore_signing_region, - ) request_dict = self._serializer.serialize_to_request( api_params, operation_model ) @@ -997,8 +995,6 @@ def _convert_to_request_dict( request_dict.pop('host_prefix', None) if headers is not None: request_dict['headers'].update(headers) - if additional_headers is not None: - request_dict['headers'].update(additional_headers) if set_user_agent_header: user_agent = self._client_config.user_agent else: diff --git a/botocore/signers.py b/botocore/signers.py index 3d82158e56..0a0d957f24 100644 --- a/botocore/signers.py +++ b/botocore/signers.py @@ -656,13 +656,25 @@ def generate_presigned_url( raise UnknownClientMethodError(method_name=client_method) operation_model = self.meta.service_model.operation_model(operation_name) + params = self._emit_api_params( + api_params=params, + operation_model=operation_model, + context=context, + ) bucket_is_arn = ArnParser.is_arn(params.get('Bucket', '')) + endpoint_url, additional_headers = self._resolve_endpoint_ruleset( + operation_model, + params, + context, + ignore_signing_region=(not bucket_is_arn), + ) request_dict = self._convert_to_request_dict( api_params=params, operation_model=operation_model, + endpoint_url=endpoint_url, context=context, + headers=additional_headers, set_user_agent_header=False, - ignore_signing_region=(not bucket_is_arn), ) # Switch out the http method if user specified it. @@ -770,14 +782,25 @@ def generate_presigned_post( # We choose the CreateBucket operation model because its url gets # serialized to what a presign post requires. operation_model = self.meta.service_model.operation_model('CreateBucket') - params = {'Bucket': bucket} + params = self._emit_api_params( + api_params={'Bucket': bucket}, + operation_model=operation_model, + context=context, + ) bucket_is_arn = ArnParser.is_arn(params.get('Bucket', '')) + endpoint_url, additional_headers = self._resolve_endpoint_ruleset( + operation_model, + params, + context, + ignore_signing_region=(not bucket_is_arn), + ) request_dict = self._convert_to_request_dict( api_params=params, operation_model=operation_model, + endpoint_url=endpoint_url, context=context, + headers=additional_headers, set_user_agent_header=False, - ignore_signing_region=(not bucket_is_arn), ) # Append that the bucket name to the list of conditions. From d8ed89de95ced2f57e35ed4f42124951dd565506 Mon Sep 17 00:00:00 2001 From: Jonas Neubert Date: Wed, 10 May 2023 16:56:28 -0600 Subject: [PATCH 7/7] bring back empty lines as per PR review comment --- botocore/signers.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/botocore/signers.py b/botocore/signers.py index 0a0d957f24..0acbf68463 100644 --- a/botocore/signers.py +++ b/botocore/signers.py @@ -668,6 +668,7 @@ def generate_presigned_url( context, ignore_signing_region=(not bucket_is_arn), ) + request_dict = self._convert_to_request_dict( api_params=params, operation_model=operation_model, @@ -794,6 +795,7 @@ def generate_presigned_post( context, ignore_signing_region=(not bucket_is_arn), ) + request_dict = self._convert_to_request_dict( api_params=params, operation_model=operation_model,