Skip to content

Commit

Permalink
simplify warning
Browse files Browse the repository at this point in the history
  • Loading branch information
davidlm committed Nov 14, 2023
1 parent 9e5c01f commit 5aab548
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 80 deletions.
3 changes: 2 additions & 1 deletion botocore/args.py
Original file line number Diff line number Diff line change
Expand Up @@ -627,9 +627,10 @@ def _construct_builtin_resolver(
credential_builtin_resolver = CredentialBuiltinResolver(
credentials,
client_config.account_id_endpoint_mode,
uses_builtin_data,
)
resolver_map = {'credentials': credential_builtin_resolver}
return EndpointBuiltinResolver(resolver_map, uses_builtin_data)
return EndpointBuiltinResolver(resolver_map)

def _build_endpoint_resolver(
self,
Expand Down
21 changes: 0 additions & 21 deletions botocore/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
# ANY KIND, either express or implied. See the License for the specific
# language governing permissions and limitations under the License.
import logging
import warnings

from botocore import waiter, xform_name
from botocore.args import ClientArgsCreator
Expand Down Expand Up @@ -1100,26 +1099,6 @@ def _resolve_endpoint_ruleset(

return endpoint_url, additional_headers

def _maybe_warn_signing_region_mismatch(self, signing_context):
legacy_signing_region = self._request_signer.region_name
signing_region = signing_context.get('region')
if (
signing_region is not None
and legacy_signing_region != signing_region
and not self._ruleset_resolver.uses_builtin_data_path
and self._ruleset_resolver.credential_scope_set
):
warnings.warn(
"Detected an endpoint resolved from a custom endpoints.json"
"file and credentials scoped to a single region: "
f"'{signing_region}'. The signing region this file has "
"resolved does not match the signing region of the client. "
"This may cause issues with request signing.\n"
f"legacy signing region: '{legacy_signing_region}'\n"
f"current signing region: '{signing_region}'\n "
f"Using '{signing_region}' to sign the request."
)

def get_paginator(self, operation_name):
"""Create a paginator for an operation.
Expand Down
41 changes: 18 additions & 23 deletions botocore/regions.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import copy
import logging
import re
import warnings
from enum import Enum

from botocore import UNSIGNED, xform_name
Expand Down Expand Up @@ -470,12 +471,15 @@ class CredentialBuiltinResolver:
'required',
)

def __init__(self, credentials, account_id_endpoint_mode):
def __init__(
self, credentials, account_id_endpoint_mode, uses_builtin_data_path
):
self._credentials = credentials
if account_id_endpoint_mode is None:
account_id_endpoint_mode = self.DEFAULT_ACCOUNT_ID_ENDPOINT_MODE
self._account_id_endpoint_mode = account_id_endpoint_mode
self._validate_account_id_endpoint_mode()
self.uses_builtin_data_path = uses_builtin_data_path

def resolve_account_id_builtin(self, builtin_configured, builtin_value):
"""Resolve the ``AWS::Auth::AccountId`` builtin."""
Expand Down Expand Up @@ -530,18 +534,8 @@ def resolve_credential_scope_builtin(
class EndpointBuiltinResolver:
"""Resolves endpoint builtins during endpoint construction."""

def __init__(self, resolver_map, uses_builtin_data_path=True):
def __init__(self, resolver_map):
self._resolver_map = resolver_map
self._uses_builtin_data_path = uses_builtin_data_path
self._credential_scope_set = False

@property
def uses_builtin_data_path(self):
return self._uses_builtin_data_path

@property
def credential_scope_set(self):
return self._credential_scope_set

def resolve(self, param_definitions, builtins):
"""Resolve endpoint builtins."""
Expand Down Expand Up @@ -573,16 +567,24 @@ def _resolve_credential_scope_builtin(self, param_definitions, builtins):
scope = credential_resolver.resolve_credential_scope_builtin(
builtin_configured, current_builtin_value
)
if scope is not None:
self._credential_scope_set = True
else:
self._credential_scope_set = False
self._maybe_warn_scope_signing(scope, builtins, credential_resolver)
builtins[scope_builtin_key] = scope

def _builtin_configured(self, param_definitions, param_name):
param_def = param_definitions.get(param_name)
return param_def is not None and param_def.builtin is not None

def _maybe_warn_scope_signing(self, scope, builtins, credential_resolver):
custom_endpoint_key = EndpointResolverBuiltins.SDK_ENDPOINT
custom_endpoint_set = builtins.get(custom_endpoint_key) is not None
uses_custom_data = credential_resolver.uses_builtin_data_path is False
if scope is not None and custom_endpoint_set and uses_custom_data:
warnings.warn(
"Detected an endpoint resolved from a custom endpoints.json"
f"file and credentials scoped to a single region: {scope}. "
"This value may not be honored during request signing."
)


class EndpointRulesetResolver:
"""Resolves endpoints using a service's endpoint ruleset"""
Expand Down Expand Up @@ -970,13 +972,6 @@ def ruleset_error_to_botocore_exception(self, ruleset_exception, params):
return InvalidEndpointConfigurationError(msg=msg)
return None

@property
def uses_builtin_data_path(self):
if self._builtin_resolver is None:
return True

return self._builtin_resolver.uses_builtin_data_path

@property
def credential_scope_set(self):
if self._builtin_resolver is None:
Expand Down
26 changes: 0 additions & 26 deletions tests/functional/test_endpoint_rulesets.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import json
from functools import lru_cache
from pathlib import Path
from unittest import mock

import pytest

Expand Down Expand Up @@ -306,28 +305,3 @@ def builtin_overwriter_handler(builtins, **kwargs):
except (ClientError, ResponseParserError):
pass
assert len(http_stubber.requests) == 0


@mock.patch('botocore.signers.RequestSigner.region_name', 'us-east-1')
@mock.patch(
'botocore.regions.EndpointRulesetResolver.auth_schemes_to_signing_ctx',
return_value=('v4', {'region': 'us-west-2'}),
)
@mock.patch(
'botocore.regions.EndpointRulesetResolver.uses_builtin_data_path', False
)
@mock.patch(
'botocore.regions.EndpointRulesetResolver.credential_scope_set', True
)
def test_signing_region_mismatch_warns(auth_schemes_mock, patched_session):
patched_session.set_credentials(
access_key='foo', secret_key='bar', token='baz', scope='us-west-2'
)
client = patched_session.create_client('iam', region_name='us-west-2')
body = b'<ListRolesResponse><ListRolesResult></ListRolesResult></ListRolesResponse>'
with ClientHTTPStubber(client) as http_stubber:
http_stubber.add_response(status=200, body=body)
with pytest.warns(
UserWarning, match='does not match the signing region'
):
client.list_roles()
28 changes: 27 additions & 1 deletion tests/unit/data/endpoints/valid-rules/aws-credential-scope.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,35 @@
"type": "string",
"builtIn": "AWS::Auth::CredentialScope",
"documentation": "The credential scope to dispatch this request, eg. `us-east-1`."
}
},
"Endpoint": {
"type": "String",
"builtIn": "SDK::Endpoint",
"required": false,
"documentation": "Override the endpoint used to send this request"
}
},
"rules": [
{
"conditions": [
{
"fn": "isSet",
"argv": [
{
"ref": "Endpoint"
}
]
}
],
"type": "endpoint",
"endpoint": {
"url": {
"ref": "Endpoint"
},
"properties": {},
"headers": {}
}
},
{
"documentation": "Template the credential scope into the URI when it is set",
"conditions": [
Expand Down
32 changes: 24 additions & 8 deletions tests/unit/test_endpoint_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -602,12 +602,13 @@ def create_ruleset_resolver(
bulitins,
credentials,
account_id_endpoint_mode,
uses_builtin_data=True,
):
service_model = Mock()
service_model.client_context_parameters = []
resolver_map = {
"credentials": CredentialBuiltinResolver(
credentials, account_id_endpoint_mode
credentials, account_id_endpoint_mode, uses_builtin_data
)
}
builtin_resolver = EndpointBuiltinResolver(resolver_map)
Expand Down Expand Up @@ -746,34 +747,30 @@ def test_required_mode_no_account_id(


@pytest.mark.parametrize(
"builtins, credentials, scope_set, expected_url",
"builtins, credentials, expected_url",
[
# scope matches region
(
BUILTINS_WITH_UNRESOLVED_CREDENTIAL_SCOPE,
CREDENTIALS_WITH_SCOPE,
True,
URL_WITH_CREDENTIAL_SCOPE,
),
# pre-resolved scope
(
BUILTINS_WITH_RESOLVED_CREDENTIAL_SCOPE,
CREDENTIALS_WITH_SCOPE,
True,
URL_WITH_OTHER_CREDENTIAL_SCOPE,
),
# no scope in credentials
(
BUILTINS_WITH_UNRESOLVED_CREDENTIAL_SCOPE,
CREDENTIALS_NO_SCOPE,
False,
URL_NO_SCOPE,
),
# no credentials
(
BUILTINS_WITH_UNRESOLVED_CREDENTIAL_SCOPE,
None,
False,
URL_NO_SCOPE,
),
],
Expand All @@ -783,7 +780,6 @@ def test_credential_scope_builtin(
credential_scope_ruleset,
builtins,
credentials,
scope_set,
expected_url,
):
resolver = create_ruleset_resolver(
Expand All @@ -794,5 +790,25 @@ def test_credential_scope_builtin(
request_context={},
call_args={},
)
assert resolver.credential_scope_set == scope_set
assert endpoint.url == expected_url


def test_credential_scope_custom_endpoints_file_warns(
operation_model_empty_context_params, credential_scope_ruleset
):
builtins = BUILTINS_WITH_UNRESOLVED_CREDENTIAL_SCOPE.copy()
endpoint_key = EndpointResolverBuiltins.SDK_ENDPOINT
builtins[endpoint_key] = "us-east-2.amazonaws.com"
resolver = create_ruleset_resolver(
credential_scope_ruleset,
builtins,
CREDENTIALS_WITH_SCOPE,
PREFERRED,
False,
)
with pytest.warns():
resolver.construct_endpoint(
operation_model=operation_model_empty_context_params,
request_context={},
call_args={},
)

0 comments on commit 5aab548

Please sign in to comment.