Repository: boto/botocore
Pull request #2797 (2022-10-27 04:59:49)

Support client context params for s3control
Context: #2740 introduced support for context params into Botocore. #2785 introduced a new way of resolving endpoints that utilizes context parameters. 

There are three types of context params: Static, dynamic, and client context params. Botocore currently intends to support _client_ context params only for the S3 services. The code in #2785 limits the use of client context params to `s3` only. This PR expands this to additionally include `s3control`. 

This PR also introduces functional test coverage that uses minimal fake models and rulesets to assert that all three types of context param are passed into the endpoint provider when expected, and not otherwise. 

codecov-commenter: # [Codecov](https://codecov.io/gh/boto/botocore/pull/2797?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boto) Report
Base: **93.57**% // Head: **93.54**% // Decreases project coverage by **`-0.02%`** :warning:
> Coverage data is based on head [(`d594326`)](https://codecov.io/gh/boto/botocore/pull/2797?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boto) compared to base [(`72df8fa`)](https://codecov.io/gh/boto/botocore/commit/72df8fa0ab6916b652655ec32409b5ac77d936b5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boto).
> Patch coverage: 100.00% of modified lines in pull request are covered.

<details><summary>Additional details and impacted files</summary>


```diff
@@             Coverage Diff             @@
##           develop    #2797      +/-   ##
===========================================
- Coverage    93.57%   93.54%   -0.03%     
===========================================
  Files           63       63              
  Lines        13300    13302       +2     
===========================================
- Hits         12445    12444       -1     
- Misses         855      858       +3     
```


| [Impacted Files](https://codecov.io/gh/boto/botocore/pull/2797?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boto) | Coverage Δ | |
|---|---|---|
| [botocore/args.py](https://codecov.io/gh/boto/botocore/pull/2797/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boto#diff-Ym90b2NvcmUvYXJncy5weQ==) | `100.00% <100.00%> (ø)` | |
| [botocore/compat.py](https://codecov.io/gh/boto/botocore/pull/2797/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boto#diff-Ym90b2NvcmUvY29tcGF0LnB5) | `91.33% <0.00%> (-1.34%)` | :arrow_down: |
| [botocore/utils.py](https://codecov.io/gh/boto/botocore/pull/2797/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boto#diff-Ym90b2NvcmUvdXRpbHMucHk=) | `78.99% <0.00%> (-0.07%)` | :arrow_down: |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boto). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boto)

</details>

[:umbrella: View full report at Codecov](https://codecov.io/gh/boto/botocore/pull/2797?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boto).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boto).


Patch:
diff --git a/botocore/args.py b/botocore/args.py
index c9338cd9b..9ae10b52c 100644
--- a/botocore/args.py
+++ b/botocore/args.py
@@ -239,7 +239,7 @@ class ClientArgsCreator:
         self._compute_connect_timeout(config_kwargs)
         s3_config = self.compute_s3_config(client_config)
 
-        is_s3_service = service_name in ['s3', 's3-control']
+        is_s3_service = self._is_s3_service(service_name)
 
         if is_s3_service and 'dualstack' in endpoint_variant_tags:
             if s3_config is None:
@@ -278,6 +278,16 @@ class ClientArgsCreator:
 
         return s3_configuration
 
+    def _is_s3_service(self, service_name):
+        """Whether the service is S3 or S3 Control.
+
+        Note that throughout this class, service_name refers to the endpoint
+        prefix, not the folder name of the service in botocore/data. For
+        S3 Control, the folder name is 's3control' but the endpoint prefix is
+        's3-control'.
+        """
+        return service_name in ['s3', 's3-control']
+
     def _compute_endpoint_config(
         self,
         service_name,
@@ -532,9 +542,11 @@ class ClientArgsCreator:
         # botocore does not support client context parameters generically
         # for every service. Instead, the s3 config section entries are
         # available as client context parameters. In the future, endpoint
-        # rulesets of services other than S3 may require client context
-        # parameters.
-        client_context = s3_config_raw if service_name_raw == 's3' else {}
+        # rulesets of services other than s3/s3control may require client
+        # context parameters.
+        client_context = (
+            s3_config_raw if self._is_s3_service(service_name_raw) else {}
+        )
         sig_version = (
             client_config.signature_version
             if client_config is not None


Test patch:
diff --git a/tests/functional/test_context_params.py b/tests/functional/test_context_params.py
new file mode 100644
index 000000000..f83327d24
--- /dev/null
+++ b/tests/functional/test_context_params.py
@@ -0,0 +1,502 @@
+# Copyright 2022 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"). You
+# may not use this file except in compliance with the License. A copy of
+# the License is located at
+#
+# http://aws.amazon.com/apache2.0/
+#
+# or in the "license" file accompanying this file. This file is
+# distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF
+# ANY KIND, either express or implied. See the License for the specific
+# language governing permissions and limitations under the License.
+
+import pytest
+
+from botocore.config import Config
+from tests import ClientHTTPStubber, mock
+
+
+# Some tests in this file require ruleset based endpoint resolution to be
+# enabled for all services (including the mock service "otherservice"). This
+# temporary fixutre should be removed once ruleset based endpoint resolution
+# is no longer limited to a subset of services.
+@pytest.fixture(autouse=True)
+def enable_endpoint_resolution_v2_for_all_services():
+    with mock.patch('botocore.client.FORCE_ENDPOINT_RESOLUTION_V2', True):
+        yield
+
+
+# fake rulesets compatible with all fake service models below
+
+FAKE_RULESET_TEMPLATE = {
+    "version": "1.0",
+    "parameters": {},
+    "rules": [
+        {
+            "conditions": [],
+            "type": "endpoint",
+            "endpoint": {
+                "url": "https://foo.bar",
+                "properties": {},
+                "headers": {},
+            },
+        }
+    ],
+}
+
+# The region param is unrelated to context parameters and used as control in
+# all test cases to ascertain that ANY EndpointProvider paramaters get
+# populated.
+REGION_PARAM = {
+    "builtIn": "AWS::Region",
+    "required": False,
+    "documentation": "",
+    "type": "String",
+}
+
+FAKE_RULESET_WITHOUT_ANY_CONTEXT_PARAMS = {
+    **FAKE_RULESET_TEMPLATE,
+    "parameters": {
+        "Region": REGION_PARAM,
+    },
+}
+
+FAKE_RULESET_WITH_CLIENT_CONTEXT_PARAM = {
+    **FAKE_RULESET_TEMPLATE,
+    "parameters": {
+        "Region": REGION_PARAM,
+        "FooClientContextParamName": {
+            "required": False,
+            "documentation": "",
+            "type": "String",
+        },
+    },
+}
+
+FAKE_RULESET_WITH_STATIC_CONTEXT_PARAM = {
+    **FAKE_RULESET_TEMPLATE,
+    "parameters": {
+        "Region": REGION_PARAM,
+        "FooStaticContextParamName": {
+            "required": False,
+            "documentation": "",
+            "type": "String",
+        },
+    },
+}
+
+FAKE_RULESET_WITH_DYNAMIC_CONTEXT_PARAM = {
+    **FAKE_RULESET_TEMPLATE,
+    "parameters": {
+        "Region": REGION_PARAM,
+        "FooDynamicContextParamName": {
+            "required": False,
+            "documentation": "",
+            "type": "String",
+        },
+    },
+}
+
+# fake models for "otherservice"
+
+FAKE_MODEL_WITHOUT_ANY_CONTEXT_PARAMS = {
+    "version": "2.0",
+    "documentation": "",
+    "metadata": {
+        "apiVersion": "2020-02-02",
+        "endpointPrefix": "otherservice",
+        "protocol": "rest-xml",
+        "serviceFullName": "Other Service",
+        "serviceId": "Other Service",
+        "signatureVersion": "v4",
+        "signingName": "otherservice",
+        "uid": "otherservice-2020-02-02",
+    },
+    "operations": {
+        "MockOperation": {
+            "name": "MockOperation",
+            "http": {"method": "GET", "requestUri": "/"},
+            "input": {"shape": "MockOperationRequest"},
+            "documentation": "",
+        },
+    },
+    "shapes": {
+        "MockOpParam": {
+            "type": "string",
+        },
+        "MockOperationRequest": {
+            "type": "structure",
+            "required": ["MockOpParam"],
+            "members": {
+                "MockOpParam": {
+                    "shape": "MockOpParam",
+                    "documentation": "",
+                    "location": "uri",
+                    "locationName": "param",
+                },
+            },
+        },
+    },
+}
+
+FAKE_MODEL_WITH_CLIENT_CONTEXT_PARAM = {
+    **FAKE_MODEL_WITHOUT_ANY_CONTEXT_PARAMS,
+    "clientContextParams": {
+        "FooClientContextParamName": {
+            "documentation": "My mock client context parameter",
+            "type": "string",
+        }
+    },
+}
+
+FAKE_MODEL_WITH_STATIC_CONTEXT_PARAM = {
+    **FAKE_MODEL_WITHOUT_ANY_CONTEXT_PARAMS,
+    "operations": {
+        "MockOperation": {
+            "name": "MockOperation",
+            "http": {"method": "GET", "requestUri": "/"},
+            "input": {"shape": "MockOperationRequest"},
+            "documentation": "",
+            "staticContextParams": {
+                "FooStaticContextParamName": {
+                    "value": "foo-static-context-param-value"
+                }
+            },
+        },
+    },
+}
+
+FAKE_MODEL_WITH_DYNAMIC_CONTEXT_PARAM = {
+    **FAKE_MODEL_WITHOUT_ANY_CONTEXT_PARAMS,
+    "shapes": {
+        "MockOpParam": {
+            "type": "string",
+        },
+        "MockOperationRequest": {
+            "type": "structure",
+            "required": ["MockOpParam"],
+            "members": {
+                "MockOpParam": {
+                    "shape": "MockOpParam",
+                    "documentation": "",
+                    "location": "uri",
+                    "locationName": "param",
+                    "contextParam": {"name": "FooDynamicContextParamName"},
+                },
+            },
+        },
+    },
+    #
+}
+
+# fake models for s3 and s3control, the only services botocore currently
+# supports client context parameters for
+
+S3_METADATA = {
+    "apiVersion": "2006-03-01",
+    "checksumFormat": "md5",
+    "endpointPrefix": "s3",
+    "globalEndpoint": "s3.amazonaws.com",
+    "protocol": "rest-xml",
+    "serviceAbbreviation": "Amazon S3",
+    "serviceFullName": "Amazon Simple Storage Service",
+    "serviceId": "S3",
+    "signatureVersion": "s3",
+    "uid": "s3-2006-03-01",
+}
+
+FAKE_S3_MODEL_WITHOUT_ANY_CONTEXT_PARAMS = {
+    **FAKE_MODEL_WITHOUT_ANY_CONTEXT_PARAMS,
+    "metadata": S3_METADATA,
+}
+
+FAKE_S3_MODEL_WITH_CLIENT_CONTEXT_PARAM = {
+    **FAKE_MODEL_WITH_CLIENT_CONTEXT_PARAM,
+    "metadata": S3_METADATA,
+}
+
+S3CONTROL_METADATA = {
+    "apiVersion": "2018-08-20",
+    "endpointPrefix": "s3-control",
+    "protocol": "rest-xml",
+    "serviceFullName": "AWS S3 Control",
+    "serviceId": "S3 Control",
+    "signatureVersion": "s3v4",
+    "signingName": "s3",
+    "uid": "s3control-2018-08-20",
+}
+
+FAKE_S3CONTROL_MODEL_WITHOUT_ANY_CONTEXT_PARAMS = {
+    **FAKE_MODEL_WITHOUT_ANY_CONTEXT_PARAMS,
+    "metadata": S3CONTROL_METADATA,
+}
+
+FAKE_S3CONTROL_MODEL_WITH_CLIENT_CONTEXT_PARAM = {
+    **FAKE_MODEL_WITH_CLIENT_CONTEXT_PARAM,
+    "metadata": S3CONTROL_METADATA,
+}
+
+
+def patch_load_service_model(
+    session, monkeypatch, service_model_json, ruleset_json
+):
+    def mock_load_service_model(service_name, type_name, api_version=None):
+        if type_name == 'service-2':
+            return service_model_json
+        if type_name == 'endpoint-rule-set-1':
+            return ruleset_json
+
+    loader = session.get_component('data_loader')
+    monkeypatch.setattr(loader, 'load_service_model', mock_load_service_model)
+
+
+@pytest.mark.parametrize(
+    'service_name,service_model,ruleset,call_should_include_ctx_param',
+    [
+        # s3
+        (
+            's3',
+            FAKE_S3_MODEL_WITH_CLIENT_CONTEXT_PARAM,
+            FAKE_RULESET_WITH_CLIENT_CONTEXT_PARAM,
+            True,
+        ),
+        (
+            's3',
+            FAKE_S3_MODEL_WITH_CLIENT_CONTEXT_PARAM,
+            FAKE_RULESET_WITHOUT_ANY_CONTEXT_PARAMS,
+            False,
+        ),
+        (
+            's3',
+            FAKE_S3_MODEL_WITHOUT_ANY_CONTEXT_PARAMS,
+            FAKE_RULESET_WITH_CLIENT_CONTEXT_PARAM,
+            False,
+        ),
+        (
+            's3',
+            FAKE_S3_MODEL_WITHOUT_ANY_CONTEXT_PARAMS,
+            FAKE_RULESET_WITHOUT_ANY_CONTEXT_PARAMS,
+            False,
+        ),
+        # s3control
+        (
+            's3control',
+            FAKE_S3CONTROL_MODEL_WITH_CLIENT_CONTEXT_PARAM,
+            FAKE_RULESET_WITH_CLIENT_CONTEXT_PARAM,
+            True,
+        ),
+        (
+            's3control',
+            FAKE_S3CONTROL_MODEL_WITH_CLIENT_CONTEXT_PARAM,
+            FAKE_RULESET_WITHOUT_ANY_CONTEXT_PARAMS,
+            False,
+        ),
+        (
+            's3control',
+            FAKE_S3CONTROL_MODEL_WITHOUT_ANY_CONTEXT_PARAMS,
+            FAKE_RULESET_WITH_CLIENT_CONTEXT_PARAM,
+            False,
+        ),
+        (
+            's3control',
+            FAKE_S3CONTROL_MODEL_WITHOUT_ANY_CONTEXT_PARAMS,
+            FAKE_RULESET_WITHOUT_ANY_CONTEXT_PARAMS,
+            False,
+        ),
+        # botocore does not currently support client context params for
+        # services other than s3 and s3-control.
+        (
+            'otherservice',
+            FAKE_MODEL_WITH_CLIENT_CONTEXT_PARAM,
+            FAKE_RULESET_WITH_CLIENT_CONTEXT_PARAM,
+            False,  # would be True for s3 and s3control
+        ),
+        (
+            'otherservice',
+            FAKE_MODEL_WITH_CLIENT_CONTEXT_PARAM,
+            FAKE_RULESET_WITHOUT_ANY_CONTEXT_PARAMS,
+            False,  # same as for s3 and s3control
+        ),
+        (
+            'otherservice',
+            FAKE_MODEL_WITHOUT_ANY_CONTEXT_PARAMS,
+            FAKE_RULESET_WITH_CLIENT_CONTEXT_PARAM,
+            False,  # same as for s3 and s3control
+        ),
+        (
+            'otherservice',
+            FAKE_MODEL_WITHOUT_ANY_CONTEXT_PARAMS,
+            FAKE_RULESET_WITHOUT_ANY_CONTEXT_PARAMS,
+            False,  # same as for s3 and s3control
+        ),
+    ],
+)
+def test_client_context_param_sent_to_endpoint_resolver(
+    monkeypatch,
+    patched_session,
+    service_name,
+    service_model,
+    ruleset,
+    call_should_include_ctx_param,
+):
+    # patch loader to return fake service model and fake endpoint ruleset
+    patch_load_service_model(
+        patched_session, monkeypatch, service_model, ruleset
+    )
+
+    # construct client using patched loader and a config object with an s3
+    # section that sets the foo_context_param to a value
+    client = patched_session.create_client(
+        service_name,
+        region_name='us-east-1',
+        config=Config(
+            s3={'foo_client_context_param_name': 'foo_context_param_value'}
+        ),
+    )
+
+    # Stub client to prevent a request from getting sent and asceertain 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:
+        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')
+
+    if call_should_include_ctx_param:
+        mock_resolve_endpoint.assert_called_once_with(
+            Region='us-east-1',
+            FooClientContextParamName='foo_context_param_value',
+        )
+    else:
+        mock_resolve_endpoint.assert_called_once_with(Region='us-east-1')
+
+
+@pytest.mark.parametrize(
+    'service_name,service_model,ruleset,call_should_include_ctx_param',
+    [
+        (
+            'otherservice',
+            FAKE_MODEL_WITH_STATIC_CONTEXT_PARAM,
+            FAKE_RULESET_WITH_STATIC_CONTEXT_PARAM,
+            True,
+        ),
+        (
+            'otherservice',
+            FAKE_MODEL_WITH_STATIC_CONTEXT_PARAM,
+            FAKE_RULESET_WITHOUT_ANY_CONTEXT_PARAMS,
+            False,
+        ),
+        (
+            'otherservice',
+            FAKE_MODEL_WITHOUT_ANY_CONTEXT_PARAMS,
+            FAKE_RULESET_WITH_STATIC_CONTEXT_PARAM,
+            False,
+        ),
+        (
+            'otherservice',
+            FAKE_MODEL_WITHOUT_ANY_CONTEXT_PARAMS,
+            FAKE_RULESET_WITHOUT_ANY_CONTEXT_PARAMS,
+            False,
+        ),
+    ],
+)
+def test_static_context_param_sent_to_endpoint_resolver(
+    monkeypatch,
+    patched_session,
+    service_name,
+    service_model,
+    ruleset,
+    call_should_include_ctx_param,
+):
+    # patch loader to return fake service model and fake endpoint ruleset
+    patch_load_service_model(
+        patched_session, monkeypatch, service_model, ruleset
+    )
+
+    # construct client using patched loader, but no special config is required
+    # for static context param to take effect
+    client = patched_session.create_client(
+        service_name, region_name='us-east-1'
+    )
+
+    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')
+
+    if call_should_include_ctx_param:
+        mock_resolve_endpoint.assert_called_once_with(
+            Region='us-east-1',
+            FooStaticContextParamName='foo-static-context-param-value',
+        )
+    else:
+        mock_resolve_endpoint.assert_called_once_with(Region='us-east-1')
+
+
+@pytest.mark.parametrize(
+    'service_name,service_model,ruleset,call_should_include_ctx_param',
+    [
+        (
+            'otherservice',
+            FAKE_MODEL_WITH_DYNAMIC_CONTEXT_PARAM,
+            FAKE_RULESET_WITH_DYNAMIC_CONTEXT_PARAM,
+            True,
+        ),
+        (
+            'otherservice',
+            FAKE_MODEL_WITH_DYNAMIC_CONTEXT_PARAM,
+            FAKE_RULESET_WITHOUT_ANY_CONTEXT_PARAMS,
+            False,
+        ),
+        (
+            'otherservice',
+            FAKE_MODEL_WITHOUT_ANY_CONTEXT_PARAMS,
+            FAKE_RULESET_WITH_DYNAMIC_CONTEXT_PARAM,
+            False,
+        ),
+    ],
+)
+def test_dynamic_context_param_sent_to_endpoint_resolver(
+    monkeypatch,
+    patched_session,
+    service_name,
+    service_model,
+    ruleset,
+    call_should_include_ctx_param,
+):
+    # patch loader to return fake service model and fake endpoint ruleset
+    patch_load_service_model(
+        patched_session, monkeypatch, service_model, ruleset
+    )
+
+    # construct client using patched loader, but no special config is required
+    # for static context param to take effect
+    client = patched_session.create_client(
+        service_name, region_name='us-east-1'
+    )
+
+    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')
+
+    if call_should_include_ctx_param:
+        mock_resolve_endpoint.assert_called_once_with(
+            Region='us-east-1',
+            FooDynamicContextParamName='mock-op-param-value',
+        )
+    else:
+        mock_resolve_endpoint.assert_called_once_with(Region='us-east-1')