Add options for s3 verification #1070
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In particular, it makes it possible to disable verification if your S3 endpoint uses self-signed certificates (not recommended). Figuring out what happens if anyone inputs undocumented argument values was a wild ride through Amazon's object-oriented python code.
|
So, being able to show the unreviewed status of this PR led to rapid improvements in our PKI and made the world a safer place. For the end verdict I'm thinking that not having this flag is an excellent safety feature |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Internals wise, this allows the S3 classes to be constructed with the same verify argument as in boto3, which allows to disable SSL verification or to pass a path to a cert bundle separate from the one obtained from environment variables.
User experience wise, having a
None | bool | strtype for an argument is awful API design, so I only exposed a boolean flag to allow users to explicitly request or disable SSL verification, with a clear statement that it is on by default. I had the factory functions do the conversion between the argument formats. A path to a certs folder is trivial to add if needed, but env variables are still a standard way to pass them.The main usecase for this is to handle self-hosted S3 endpoints with self-signed certificates until you have proper certs management set up.