Add Param to to s3_object module to enforce SigV4 for get operations #1014
Conversation
Docs Build 📝Thank you for contribution!✨ This PR has been merged and your docs changes will be incorporated when they are next published. |
ansibullbot
added
community_review
feature
There was a problem hiding this comment.
@zollo Thank you for working on this. Left some minor suggestions.
plugins/modules/s3_object.py
Outdated
| @@ -97,6 +97,12 @@ | |||
| - Keyname of the object inside the bucket. | |||
| - Can be used to create "virtual directories", see examples. | |||
| type: str | |||
| sig_v4: | |||
| description: | |||
| - Forces the Boto SDK to use Signature Version 4 | |||
There was a problem hiding this comment.
Please add version_added: 5.0.0
plugins/modules/s3_object.py
Outdated
| @@ -97,6 +97,12 @@ | |||
| - Keyname of the object inside the bucket. | |||
| - Can be used to create "virtual directories", see examples. | |||
| type: str | |||
| sig_v4: | |||
| description: | |||
| - Forces the Boto SDK to use Signature Version 4 | |||
There was a problem hiding this comment.
| - Forces the Boto SDK to use Signature Version 4 | |
| - Forces the Boto SDK to use Signature Version 4. |
plugins/modules/s3_object.py
Outdated
| sig_v4: | ||
| description: | ||
| - Forces the Boto SDK to use Signature Version 4 | ||
| - Only applies to get modes, I(mode=get), I(mode=getstr), I(mode=geturl) |
There was a problem hiding this comment.
| - Only applies to get modes, I(mode=get), I(mode=getstr), I(mode=geturl) | |
| - Only applies to get modes, I(mode=get), I(mode=getstr), I(mode=geturl). |
| @@ -0,0 +1,2 @@ | |||
| minor_changes: | |||
There was a problem hiding this comment.
Changelog fragments must be renamed to changelogs/fragments/1014-add.......
| @@ -0,0 +1,2 @@ | |||
| minor_changes: | |||
| - s3_object - added the ``sig_v4`` paramater, enbling the user to opt in to signature version 4 for download/get operations. (https://github.com/ansible-collections/amazon.aws/issues/1013) | |||
There was a problem hiding this comment.
| - s3_object - added the ``sig_v4`` paramater, enbling the user to opt in to signature version 4 for download/get operations. (https://github.com/ansible-collections/amazon.aws/issues/1013) | |
| - s3_object - added the ``sig_v4`` paramater, enbling the user to opt in to signature version 4 for download/get operations (https://github.com/ansible-collections/amazon.aws/issues/1014). |
|
@zollo we will release 5.0.0 soon and I only tiny cosmetic issue. It would be great if we can include your PR. Can you push an update? |
|
@goneri - absolutely, sorry for the delay, i'll complete this within the hour. |
|
@zollo, no worries and thanks! |
plugins/modules/s3_object.py
Outdated
| description: | ||
| - Forces the Boto SDK to use Signature Version 4. | ||
| - Only applies to get modes, I(mode=get), I(mode=getstr), I(mode=geturl). | ||
| default: false |
There was a problem hiding this comment.
What prevent us from turning this True by default?
There was a problem hiding this comment.
@goneri I have no issues with that - I was worried about it being a potential breaking change but on second thought it's just a URL. I can push this change right away.
There was a problem hiding this comment.
It more likes a question at this stage :-). I'm not sure if this can break people's playbooks. If we switch this True by default we will need to adjust the changelog fragment to mention the new behaviour.
@jillr, @alinabuzachis, @tremble, what do you think?
There was a problem hiding this comment.
I think v4 has been set as the default by now but this will require research to confirm, and if that is true in all regions. We also need to understand if there are any cases where this could break requests for older buckets or objects, and ensure we do the right thing for non-AWS S3-like APIs (such as Ceph RGW).
There was a problem hiding this comment.
For the AWS-like APIs there's code in there that bypasses the v4 signatures. Definitely needs a changelog entry though if it's being enabled by default.
If AWS supports it on all calls, I'd be tempted to enable it by default everywhere rather than doing a per-mode test
There was a problem hiding this comment.
Sigv4 should be the default signing algorithm for everything. See Amazon's sigv2 deprecation plan
There was a problem hiding this comment.
All, upon further research it appears that this is actually the default in recent versions of Boto.
https://boto3.amazonaws.com/v1/documentation/api/latest/guide/configuration.html
For the Config object, signature_version='s3v4' is the default if the param isn't passed, specifying signature_version='s3' would force SigV2.
This PR may be completely unnecessary.
There was a problem hiding this comment.
Ok! Had a chat with my team (@phene and others) and here's what we came up with:
- If the region supports v2, Boto will default to it: https://github.com/boto/botocore/blob/73ce827de52ec52fd2f4e03dc2cdc105627edbfd/botocore/client.py#L425
Considering that the default behavior is to use V2 if the region has it enabled, otherwise use V4 - my opinion would be to allow the Ansible user to override this behavior in the event that they are deploying an a region with V2 enabled but would rather use V4.
Example: AWS US Govcloud regions appear to support V2 and V4 so in this case, Boto would use V2.
I would set the default to False and rename the param to force_sig_v4 and update the description to indicate that this only needs to be used if operating in a region where Boto's automatic detection would default to V2.
There was a problem hiding this comment.
It's also worth noting that in aws-us-gov, we want to force v4 because v2 is not FIPS compliant (as it uses SHA1).
|
recheck |
|
regate |
SUMMARY
This pull request adds a parameter to the
s3_objectmodule that enables users to force/require the Boto SDK to use SigV4 for get operations.Fixes #1013
ISSUE TYPE
COMPONENT NAME
s3_object
ADDITIONAL INFORMATION
N/A