Add support for generating a pre-signed S3 PUT url
Issue #, if available: #3050
Description of changes:
This adds support for aws s3 presign-put per the desired design.
The design mentioned having presign become an alias for presign-get - I've left this out for now because it's not dependent on this PR so should be doable in a follow up PR (I don't mind doing it in this one, but for now am focusing on the core function).
The design also mentioned having presign-post which I've also left out as from what I can tell that requires significantly more work? I would be interested in exploring supporting that too as it sounds more powerful because you can pass a policy, but I'd prefer landing it separately given how PR should be ready to go and be usable without it.
Finally, I've got a test drafted up but need some guidance on how to handle doing a PUT request as I can't find any examples elsewhere in the codebase and am not versed in Python enough to feel confident in picking an approach (I'm hoping that six has a method similar to it's urlopen but haven't been able to find anything so far).
This is the draft of the test:
class TestPresignPutCommand(BaseS3IntegrationTest):
def test_can_upload_presigned_url(self):
bucket_name = _SHARED_BUCKET
file_contents = b'this is foo.txt'
p = aws('s3 presign-put s3://%s/foo.txt' % (bucket_name,))
self.assert_no_errors(p)
url = p.stdout.strip()
# todo: make PUT request
# Make sure object is in bucket.
self.assertTrue(self.key_exists(bucket_name, key_name='foo.txt'))
self.assertEqual(
self.get_key_contents(bucket_name, key_name='foo.txt'),
file_contents)
self.assertEqual(
self.content_type_for_key(bucket_name, key_name='foo.txt'),
'text/plain')
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
The CI failures seem unrelated to my change
ERROR: Package 'botocore' requires a different Python: 3.6.15 not in '>=3.7'
& (on Windows)
'pytest' is not recognized as an internal or external command
Let me know if there's anything I need to do (e.g. rebase to pull in a fix)
The CI failures seem unrelated to my change
ERROR: Package 'botocore' requires a different Python: 3.6.15 not in '>=3.7'
& (on Windows)
'pytest' is not recognized as an internal or external command
Let me know if there's anything I need to do (e.g. rebase to pull in a fix)
Should pass if you update your branch to latest base path. They removed Python 3.6 from the testing matrix now.
Codecov Report
Attention: 9 lines in your changes are missing coverage. Please review.
Comparison is base (
6e0239d) 0.08% compared to head (927303f) 92.82%.
:exclamation: Current head 927303f differs from pull request most recent head 4111106. Consider uploading reports for the commit 4111106 to get more accurate results
Additional details and impacted files
@@ Coverage Diff @@
## develop #6993 +/- ##
============================================
+ Coverage 0.08% 92.82% +92.74%
============================================
Files 208 204 -4
Lines 16629 16344 -285
============================================
+ Hits 14 15172 +15158
+ Misses 16615 1172 -15443
| Files | Coverage Δ | |
|---|---|---|
| awscli/customizations/s3/s3.py | 100.00% <ø> (+100.00%) |
:arrow_up: |
| awscli/customizations/s3/subcommands.py | 95.56% <40.00%> (+95.56%) |
:arrow_up: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@justin-kidman-axomic thanks for the heads up - I've rebased my branch, so now someone just needs to approve running the workflow
I want to use your fork and test this new command. what is the best course of action?
[Update]: Oh, I got my answer, the new binary is on ./bin/aws
I noticed that this patch is for an older version of the CLI, that dont yet uses Signature Version 4, so maybe this is the main reason that the patch didnt got merged :(
@fczuardi this patch hasn't been merged as its still waiting to be reviewed; sigv4 has been out for quite a while and even mentioned in the description for this command so my expectation is it does support it, but if that was a blocker it would be requested in a review (which just has not happened yet).
@fczuardi if it helps, I've rebased this off latest develop so it should be more up to date
@kdaily @tim-finnigan it would be good if we could move this PR along - could I at least get the workflows approved to know if CI is passing?