git cl upload for Gerrit: warn if uploader is not CL owner.
R=agable@chromium.org
BUG=653172
Change-Id: I141a1422a8f526040fe80522a41d14e2cd8cfc07
Reviewed-on: https://chromium-review.googlesource.com/458421
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
Reviewed-by: Aaron Gable <agable@chromium.org>
diff --git a/git_cl.py b/git_cl.py
index f001974..7122d1a 100755
--- a/git_cl.py
+++ b/git_cl.py
@@ -1540,7 +1540,7 @@
# expensive hooks if uploading is likely to fail anyway. Passing these
# checks does not guarantee that uploading will not fail.
self._codereview_impl.EnsureAuthenticated(force=options.force)
- self._codereview_impl.EnsureCanUploadPatchset()
+ self._codereview_impl.EnsureCanUploadPatchset(force=options.force)
# Apply watchlists on upload.
change = self.GetChange(base_branch, None)
@@ -1809,14 +1809,17 @@
"""
raise NotImplementedError()
- def EnsureCanUploadPatchset(self):
+ def EnsureCanUploadPatchset(self, force):
"""Best effort check that uploading isn't supposed to fail for predictable
reasons.
This method should raise informative exception if uploading shouldn't
proceed.
+
+ Arguments:
+ force: whether to skip confirmation questions.
"""
- pass
+ raise NotImplementedError()
def CMDUploadChange(self, options, args, change):
"""Uploads a change to codereview."""
@@ -1873,6 +1876,10 @@
if refresh:
authenticator.get_access_token()
+ def EnsureCanUploadPatchset(self, force):
+ # No checks for Rietveld because we are deprecating Rietveld.
+ pass
+
def FetchDescription(self, force=False):
issue = self.GetIssue()
assert issue
@@ -2383,19 +2390,14 @@
cookie_auth.get_netrc_path(),
cookie_auth.get_new_password_message(git_host)))
- def EnsureCanUploadPatchset(self):
- """Best effort check that uploading isn't supposed to fail for predictable
- reasons.
-
- This method should raise informative exception if uploading shouldn't
- proceed.
- """
+ def EnsureCanUploadPatchset(self, force):
if not self.GetIssue():
return
# Warm change details cache now to avoid RPCs later, reducing latency for
# developers.
- self.FetchDescription()
+ self._GetChangeDetail(
+ ['DETAILED_ACCOUNTS', 'CURRENT_REVISION', 'CURRENT_COMMIT'])
status = self._GetChangeDetail()['status']
if status in ('MERGED', 'ABANDONED'):
@@ -2403,6 +2405,27 @@
(self.GetIssueURL(),
'submitted' if status == 'MERGED' else 'abandoned'))
+ if gerrit_util.GceAuthenticator.is_gce():
+ return
+ cookies_user = gerrit_util.CookiesAuthenticator().get_auth_email(
+ self._GetGerritHost())
+ if self.GetIssueOwner() == cookies_user:
+ return
+ logging.debug('change %s owner is %s, cookies user is %s',
+ self.GetIssue(), self.GetIssueOwner(), cookies_user)
+ # Maybe user has linked accounts or smth like that,
+ # so ask what Gerrit thinks of this user.
+ details = gerrit_util.GetAccountDetails(self._GetGerritHost(), 'self')
+ if details['email'] == self.GetIssueOwner():
+ return
+ if not force:
+ print('WARNING: change %s is owned by %s, but you authenticate to Gerrit '
+ 'as %s.\n'
+ 'Uploading may fail due to lack of permissions.' %
+ (self.GetIssue(), self.GetIssueOwner(), details['email']))
+ confirm_or_exit(action='upload')
+
+
def _PostUnsetIssueProperties(self):
"""Which branch-specific properties to erase when unsetting issue."""
return ['gerritsquashhash']