Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: public access prevention #304

Merged
merged 13 commits into from Jun 30, 2021
Merged

feat: public access prevention #304

merged 13 commits into from Jun 30, 2021

Conversation

shaffeeullah
Copy link
Contributor

@shaffeeullah shaffeeullah commented Nov 9, 2020

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #312 🦕

@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Nov 9, 2020
@product-auto-label product-auto-label bot added the api: storage Issues related to the googleapis/python-storage API. label Nov 9, 2020
Copy link
Contributor

@tritone tritone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally looks good-- my biggest question is whether we should use some kind of enum or constant for the values.

google/cloud/storage/bucket.py Outdated Show resolved Hide resolved
google/cloud/storage/bucket.py Outdated Show resolved Hide resolved
tests/system/test_system.py Outdated Show resolved Hide resolved
tests/system/test_system.py Outdated Show resolved Hide resolved
google/cloud/storage/bucket.py Outdated Show resolved Hide resolved
google/cloud/storage/bucket.py Show resolved Hide resolved
google/cloud/storage/bucket.py Show resolved Hide resolved
tests/system/test_system.py Outdated Show resolved Hide resolved
@tseaver
Copy link
Contributor

tseaver commented Nov 11, 2020

The systest failures seem to indicate that the feature is not working as expected. Do we need to get the CI project added to a whitelist?

@shaffeeullah
Copy link
Contributor Author

The systest failures seem to indicate that the feature is not working as expected. Do we need to get the CI project added to a whitelist?

The backend feature still has some bugs that are being resolved. The current failure is expected at this time.

google/cloud/storage/bucket.py Outdated Show resolved Hide resolved
google/cloud/storage/bucket.py Outdated Show resolved Hide resolved
google/cloud/storage/bucket.py Show resolved Hide resolved
google/cloud/storage/bucket.py Show resolved Hide resolved
@google-cla
Copy link

google-cla bot commented Nov 16, 2020

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

1 similar comment
@google-cla
Copy link

google-cla bot commented Nov 16, 2020

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added cla: no This human has *not* signed the Contributor License Agreement. and removed cla: yes This human has signed the Contributor License Agreement. labels Nov 16, 2020
@google-cla
Copy link

google-cla bot commented Nov 16, 2020

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@google-cla
Copy link

google-cla bot commented Nov 16, 2020

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

2 similar comments
@google-cla
Copy link

google-cla bot commented Dec 22, 2020

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@google-cla
Copy link

google-cla bot commented Dec 23, 2020

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@shaffeeullah shaffeeullah added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Dec 23, 2020
@shaffeeullah shaffeeullah marked this pull request as ready for review December 23, 2020 18:16
@shaffeeullah shaffeeullah requested a review from a team December 23, 2020 18:16
@shaffeeullah
Copy link
Contributor Author

Hi @tseaver , can you please sign the CLA? See #304 (comment)

@shaffeeullah
Copy link
Contributor Author

Hi @tseaver , can you please sign the CLA? See #304 (comment)

@tseaver friendly bump

@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@andrewsg
Copy link
Contributor

Tres has signed the CLA and submitted other PRs with the same email address without issue, so I don't know why googlebot is complaining here.

@andrewsg
Copy link
Contributor

andrewsg commented Jan 12, 2021

@shaffeeullah This seems to be an issue with Github adding Tres to the contributor list automatically, with different user details than he normally uses based on the git config in his actual computer.

Tres definitely is a CLA signer. I conferred with Frank and he suggested you rebase to remove Tres from the contributor list and force-push. Please message me on chat if you need help wrangling git here. And make a backup of your branch just in case.

Copy link
Contributor

@tritone tritone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor docs comment, otherwise looks good!

google/cloud/storage/bucket.py Show resolved Hide resolved
@google-cla
Copy link

google-cla bot commented Jun 8, 2021

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

1 similar comment
@google-cla
Copy link

google-cla bot commented Jun 8, 2021

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@shaffeeullah
Copy link
Contributor Author

Public access prevention rollout has been delayed due to a bug surfaced during Googler preview. I will keep this PR updated as I learn new release timeline details.

@shaffeeullah
Copy link
Contributor Author

@andrewsg This feature can now be merged and released. Are you able to help me with this?

@tritone tritone removed do not merge Indicates a pull request not ready for merge, due to either quality or timing. status: blocked Resolving the issue is dependent on other work. labels Jun 29, 2021
@tseaver
Copy link
Contributor

tseaver commented Jun 30, 2021

@shaffeeullah I can help fix the merge, since my PR refactoring the system tests is what broke your new ones.

@google-cla
Copy link

google-cla bot commented Jun 30, 2021

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added cla: no This human has *not* signed the Contributor License Agreement. and removed cla: yes This human has signed the Contributor License Agreement. labels Jun 30, 2021
@tseaver
Copy link
Contributor

tseaver commented Jun 30, 2021

@googlebot I consent.

@google-cla
Copy link

google-cla bot commented Jun 30, 2021

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Jun 30, 2021
@tseaver tseaver added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 30, 2021
@google-cla
Copy link

google-cla bot commented Jun 30, 2021

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@shaffeeullah
Copy link
Contributor Author

Thanks, @tseaver !! @andrewsg @tseaver If the tests pass, would it be possible to release this today?

@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 30, 2021
@tseaver
Copy link
Contributor

tseaver commented Jun 30, 2021

@shaffeeullah Yes, we should be able to make a release. @cojenco, are any of your open PRs at a state where they should be merged before a release?

@tseaver tseaver merged commit e3e57a9 into googleapis:master Jun 30, 2021
@shaffeeullah shaffeeullah deleted the publicaccessprevention branch June 30, 2021 15:26
@cojenco
Copy link
Contributor

cojenco commented Jun 30, 2021

Thanks @tseaver and @shaffeeullah! I'll be merging #480 shortly and trying to squeeze in #484 this morning as well for the release.

@tseaver
Copy link
Contributor

tseaver commented Jun 30, 2021

@shaffeeullah Release is in progress: PR #485

cojenco pushed a commit to cojenco/python-storage that referenced this pull request Oct 13, 2021
cojenco pushed a commit to cojenco/python-storage that referenced this pull request Oct 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: storage Issues related to the googleapis/python-storage API. cla: yes This human has signed the Contributor License Agreement. external This issue is blocked on a bug with the actual product.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add support for PublicAccessPrevention
7 participants