feature: V4 Post policies by IlyaFaer · Pull Request #87 · googleapis/python-storage

IlyaFaer · 2020-03-16T11:18:59Z

Towards #8

This reverts commit f56440b.

google/cloud/storage/_signing.py

google/cloud/storage/client.py

tests/unit/test_client.py

IlyaFaer · 2020-03-16T15:10:27Z

@frankyn, I've pushed a real PR. Only system tests are failing with Anonymous caller does not have storage.objects.create access to <bucket-name>/<file-name> (the same Jonathan Lui got)

Everything except this seems to be fine

frankyn · 2020-03-16T17:02:52Z

Thanks @IlyaFaer, I'll review. I'll update group in V4 Sig Hangouts Chat.

frankyn

Small nits

google/cloud/storage/_signing.py

google/cloud/storage/client.py

frankyn

Thanks for your patience @IlyaFaer, added a few more comments.

google/cloud/storage/_signing.py

google/cloud/storage/client.py

frankyn

One more nit, and will ping @crwilcox for his Python expertise.

google/cloud/storage/client.py

…storage into v4_post_policies

google/cloud/storage/client.py

IlyaFaer · 2020-03-31T14:29:40Z

@frankyn, @crwilcox, I've moved conformance tests into this PR. I assume you'd like me to add the conformance tests data as well!? Locally I've added policy conformance tests data into url_signer_v4_test_data.json, and used it while running the tests. Is it a correct place?

frankyn · 2020-03-31T14:57:09Z

@IlyaFaer yes, keep it in the same file to reduce complexity of conformance tests.

frankyn · 2020-03-31T15:14:02Z

@IlyaFaer could you update conformance tests to follow the exact format of source conformance tests: https://github.com/googleapis/conformance-tests/blob/master/storage/v1/v4_signatures.json

It can be a subsequent PR to reduce complexity for this one. I'm trying to prevent snowflakes.

frankyn

I missed a few nits in yesterday's review. Thanks for consolidating conformance tests @IlyaFaer

google/cloud/storage/client.py

frankyn

Few more nits.

tests/unit/test_client.py

google/cloud/storage/client.py

frankyn

LGTM, pending KokoroCI passing and approval from @crwilcox

IlyaFaer · 2020-03-31T16:10:13Z

follow the exact format of source conformance tests

@frankyn, you mean, use these signingV4Tests and postPolicyV4Tests fields on a first level of the json? That's not a problem, though I assume some code should be changed in test__signing.py as well, so yes, that probably should be done in another PR.

@crwilcox, @frankyn, I think, I saw random conformance tests are flakely failing in Python 2.7 and 3.5 while it all was in progress. I'll take a closer look.

frankyn · 2020-03-31T16:12:15Z

Hi @IlyaFaer,

@frankyn, you mean, use these signingV4Tests and postPolicyV4Tests fields on a first level of the json? That's not a problem, though I assume some code should be changed in test__signing.py as well, so yes, that probably should be done in another PR.

Yes, that's correct, could you please open a tracking issue for it as well? Thank you.

I think, I saw random conformance tests are flakely failing in Python 2.7 and 3.5. Will take a closer look.

Conformance tests should not have flakes, please keep us posted.

crwilcox · 2020-03-31T16:42:30Z

It seems there are a few kokoro failures. @IlyaFaer are these expected or is this work to do yet?

frankyn · 2020-03-31T23:27:40Z

@crwilcox, @IlyaFaer raised that are some flakes. It looks like system tests and not conformance tests, but I might be incorrect. Pending follow-up.

Thanks for reviewing this PR @crwilcox!

IlyaFaer · 2020-04-01T08:46:59Z

@frankyn, @crwilcox, I've found out what was causing flaky: dict.items() returns result in different order sometimes:

So I've added sorted() for fields - didn't detect any new failing on 2.7/3.5 so far.

I've fixed system tests as well (they were failing with Anonymous requests are not allowed for some time, but now they're okay, so I've fixed a couple of my bads).

I hope nothing is forgotten. All checks are green now 🎉

crwilcox · 2020-04-01T17:55:50Z

Awesome. Yep @IlyaFaer ordered dicts weren't in 2.7 at all or even older versions of 3.

* feat: add POST policies building method * add comments, ignoring x-ignore fields and required fields validation * fix docs style, add virtual hosted style URLs * add bucket_bound_hostname support * cosmetic changes * add unit tests * Revert "add unit tests" This reverts commit f56440b. * add few lines from the old implementation for consistency * add some system tests * move system tests into separate class * fix credentials scope URL mistake * fix unit tests * fix algorithm name * add an example * add access token support * add credentials as an argument * rename method * add conformance tests into client unit tests * align conformance tests with test data * add an ability to set expiration as integer * update conformance tests to avoid problems with json spaces and timestamp Z-symbol violation * update implementation to avoid Z symbol isoformat violation and json whitespaces encoding * fix error with bounded hostnames * fix problem with bounded hostnames in implementation * fix conformance tests * fix problems: ascii encoding of signature and fields order * change asserts order * fix conformance tests * fix encoding issues * cosmetic changes and adding conformance tests * fix russion "C" letter in comment * add conformance tests data * cosmetic changes * cosmetic changes * add fields sorting * fix system tests Co-authored-by: Frank Natividad <frankyn@users.noreply.github.com>

IlyaFaer added 9 commits March 11, 2020 15:42

feat: add POST policies building method

447310f

add comments, ignoring x-ignore fields and required fields validation

fa09133

fix docs style, add virtual hosted style URLs

6818fb6

add bucket_bound_hostname support

c216a59

cosmetic changes

b01c801

add unit tests

f56440b

Revert "add unit tests"

dc7028d

This reverts commit f56440b.

add few lines from the old implementation for consistency

0a442b4

add some system tests

4982ad7

IlyaFaer added the type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. label Mar 16, 2020

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Mar 16, 2020

IlyaFaer added 4 commits March 16, 2020 15:02

move system tests into separate class

333e212

fix credentials scope URL mistake

173c3f6

fix unit tests

1c80907

fix algorithm name

3f15f2e

IlyaFaer commented Mar 16, 2020

View reviewed changes

google/cloud/storage/_signing.py Show resolved Hide resolved

google/cloud/storage/client.py Outdated Show resolved Hide resolved

tests/unit/test_client.py Show resolved Hide resolved

IlyaFaer requested a review from frankyn March 16, 2020 15:07

IlyaFaer marked this pull request as ready for review March 16, 2020 15:07

frankyn requested a review from crwilcox March 17, 2020 06:01

frankyn suggested changes Mar 17, 2020

View reviewed changes

google/cloud/storage/_signing.py Outdated Show resolved Hide resolved

google/cloud/storage/client.py Outdated Show resolved Hide resolved

google/cloud/storage/client.py Show resolved Hide resolved

add an example

9acc3d7

frankyn suggested changes Mar 20, 2020

View reviewed changes

google/cloud/storage/_signing.py Outdated Show resolved Hide resolved

google/cloud/storage/client.py Outdated Show resolved Hide resolved

google/cloud/storage/client.py Show resolved Hide resolved

google/cloud/storage/client.py Show resolved Hide resolved

add access token support

b60c45e

frankyn suggested changes Mar 23, 2020

View reviewed changes

google/cloud/storage/client.py Show resolved Hide resolved

frankyn and others added 3 commits March 22, 2020 22:30

Merge branch 'master' into v4_post_policies

77ec997

add credentials as an argument

b6c4106

Merge branch 'v4_post_policies' of https://github.com/q-logic/python-…

32b2f73

…storage into v4_post_policies

IlyaFaer added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 23, 2020

frankyn reviewed Mar 31, 2020

View reviewed changes

google/cloud/storage/client.py Outdated Show resolved Hide resolved

frankyn mentioned this pull request Mar 31, 2020

[storage] feat: add post policy sample GoogleCloudPlatform/python-docs-samples#3231

Merged

4 tasks

cosmetic changes and adding conformance tests

8506d83

IlyaFaer mentioned this pull request Mar 31, 2020

add conformance tests into client unit tests MaxxleLLC/python-storage#9

Merged

fix russion "C" letter in comment

fd31846

add conformance tests data

9048cc5

frankyn suggested changes Mar 31, 2020

View reviewed changes

google/cloud/storage/client.py Show resolved Hide resolved

google/cloud/storage/client.py Outdated Show resolved Hide resolved

cosmetic changes

81759b2

IlyaFaer added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 31, 2020

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 31, 2020

frankyn suggested changes Mar 31, 2020

View reviewed changes

tests/unit/test_client.py Outdated Show resolved Hide resolved

google/cloud/storage/client.py Outdated Show resolved Hide resolved

cosmetic changes

8fcb8bf

frankyn approved these changes Mar 31, 2020

View reviewed changes

IlyaFaer mentioned this pull request Mar 31, 2020

Update conformance tests to fit the new format #92

Closed

crwilcox approved these changes Mar 31, 2020

View reviewed changes

IlyaFaer added 2 commits April 1, 2020 10:42

add fields sorting

7b1406e

fix system tests

16daedd

crwilcox merged commit b451e2d into googleapis:master Apr 1, 2020

release-please bot mentioned this pull request Jan 12, 2022

chore(main): release 3.0.0 #688

Closed

Conversation

IlyaFaer commented Mar 16, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

IlyaFaer commented Mar 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

frankyn commented Mar 16, 2020

Uh oh!

frankyn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

frankyn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

frankyn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

IlyaFaer commented Mar 31, 2020

Uh oh!

frankyn commented Mar 31, 2020

Uh oh!

frankyn commented Mar 31, 2020

Uh oh!

frankyn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

frankyn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

frankyn left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

IlyaFaer commented Mar 31, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

frankyn commented Mar 31, 2020

Uh oh!

crwilcox commented Mar 31, 2020

Uh oh!

frankyn commented Mar 31, 2020

Uh oh!

IlyaFaer commented Apr 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

crwilcox commented Apr 1, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

IlyaFaer commented Mar 16, 2020 •

edited

Loading

frankyn left a comment •

edited

Loading

IlyaFaer commented Mar 31, 2020 •

edited

Loading

IlyaFaer commented Apr 1, 2020 •

edited

Loading