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鈥檒l 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

Conversation

@shaffeeullah
Copy link
Member

@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 馃

Copy link
Contributor

@tritone tritone left a comment

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
@tritone tritone requested a review from andrewsg Nov 10, 2020
google/cloud/storage/bucket.py Outdated Show resolved Hide resolved
@shaffeeullah shaffeeullah requested a review from tritone Nov 11, 2020
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 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
Member Author

@shaffeeullah shaffeeullah 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?

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

@shaffeeullah shaffeeullah requested a review from tseaver Nov 16, 2020
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 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 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 and removed cla: yes labels Nov 16, 2020
@google-cla
Copy link

@google-cla 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.

@shaffeeullah shaffeeullah requested a review from tseaver Nov 16, 2020
@google-cla
Copy link

@google-cla 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 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 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 marked this pull request as ready for review Dec 23, 2020
@shaffeeullah shaffeeullah requested a review from Dec 23, 2020
@shaffeeullah
Copy link
Member Author

@shaffeeullah shaffeeullah commented Dec 23, 2020

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

@shaffeeullah
Copy link
Member Author

@shaffeeullah shaffeeullah commented Jan 12, 2021

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

@tseaver friendly bump

@googlebot
Copy link

@googlebot googlebot commented Jan 12, 2021

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

@andrewsg andrewsg commented Jan 12, 2021

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 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

One minor docs comment, otherwise looks good!

google/cloud/storage/bucket.py Show resolved Hide resolved
@tseaver
Copy link
Contributor

@tseaver tseaver commented Jun 8, 2021

@shaffeeullah Works for me!

@google-cla
Copy link

@google-cla 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 google-cla bot commented Jun 8, 2021

CLAs look good, thanks!

鈩癸笍 Googlers: Go here for more info.

tseaver
tseaver approved these changes Jun 8, 2021
@shaffeeullah
Copy link
Member Author

@shaffeeullah shaffeeullah commented Jun 11, 2021

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
Member Author

@shaffeeullah shaffeeullah commented Jun 28, 2021

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

@tseaver
Copy link
Contributor

@tseaver 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 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 and removed cla: yes labels Jun 30, 2021
@tseaver
Copy link
Contributor

@tseaver tseaver commented Jun 30, 2021

@googlebot I consent.

@google-cla
Copy link

@google-cla google-cla bot commented Jun 30, 2021

CLAs look good, thanks!

鈩癸笍 Googlers: Go here for more info.

@google-cla
Copy link

@google-cla google-cla bot commented Jun 30, 2021

CLAs look good, thanks!

鈩癸笍 Googlers: Go here for more info.

@shaffeeullah
Copy link
Member Author

@shaffeeullah shaffeeullah commented Jun 30, 2021

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

@tseaver
Copy link
Contributor

@tseaver 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
5 checks passed
@shaffeeullah shaffeeullah deleted the publicaccessprevention branch Jun 30, 2021
@cojenco
Copy link
Contributor

@cojenco 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 tseaver commented Jun 30, 2021

@shaffeeullah Release is in progress: PR #485

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

7 participants