Implement opt-in unsigned requests for clients. #448
Conversation
This change makes it possible to override the signature version at client
creation time using a parameter. It also introduces the concept of an
advanced configuration object for clients:
```python
import botocore
from botocore.client import Config
config = Config(signature_version=botocore.UNSIGNED)
s3 = session.create_client('s3', config=config)
```
This does **not** move any existing parameters into the advanced
configuration object, and thus is not yet a breaking change. It
does include the following:
* Adds `botocore.UNSIGNED` as a sentinel to disable signing
* Updates `botocore.handlers.disable_signing` to use `botocore.UNSIGNED`
* Adds a `botocore.client.Config` object to store signature version
* Modifies client creation to use this new config
* Updated / new unit tests
| @@ -367,6 +367,15 @@ def test_credential_provider_not_called_when_creds_provided(self): | |||
| "explicit credentials were provided to the " | |||
| "create_client call.") | |||
|
|
|||
| @mock.patch('botocore.client.ClientCreator') | |||
| def test_config_passed_to_client_creator(self, client_creator): | |||
| config = mock.Mock() | |||
There was a problem hiding this comment.
We've discussed in the past not mocking value objects. Let's just use the actual Config object here.
| @@ -86,7 +87,7 @@ def sign(self, operation_name, request): | |||
| signature_version=signature_version, request_signer=self) | |||
|
|
|||
| # Sign the request if the signature version isn't None or blank | |||
| if signature_version: | |||
| if signature_version != botocore.UNSIGNED: | |||
There was a problem hiding this comment.
Will this affect the CLI? We set the endpoint's signature version to None. You can check real quick by running this test: https://github.com/aws/aws-cli/blob/develop/tests/integration/customizations/s3/test_plugin.py#L1594
There was a problem hiding this comment.
Given it's now a sentinel, we should be using identity checks instead of equality check (signature_version is not botocore.UNSIGNED)
|
This looks good to me. 🚢 If the integration test that I pointed out does fail, a subsequent pull request will have to be made to change |
|
@kyleknap good call. I've created an associated pull request with the CLI for this. After it's been reviewed I'll merge in both. |
| @@ -118,7 +119,8 @@ def test_disable_signing(self): | |||
| # Returning a blank string from choose-signer disabled signing! | |||
There was a problem hiding this comment.
This comment is no longer accurate.
|
Looks good, just a couple small things. |
Implement opt-in unsigned requests for clients.
This change makes it possible to override the signature version at client
creation time using a parameter. It also introduces the concept of an
advanced configuration object for clients:
This does not move any existing parameters into the advanced
configuration object, and thus is not yet a breaking change. It
does include the following:
botocore.UNSIGNEDas a sentinel to disable signingbotocore.handlers.disable_signingto usebotocore.UNSIGNEDbotocore.client.Configobject to store signature versioncc @jamesls @kyleknap