Commit b250ec16 authored by Vadim Shtayura's avatar Vadim Shtayura Committed by Commit Bot

[git_cl] Don't check .gitcookies when running on LUCI.

This CL basically replaces "skip if is_gce" with "skip if default auth method
is not gitcookies".

This presumable makes 'git cl' work on LUCI in a consistent manner. Before, it
worked only if the LUCI bot happened to also be GCE bot.

R=tandrii@chromium.org
BUG=891755

Change-Id: I2caa219a4082438a5e026e728bfb62f46a0c80fd
Reviewed-on: https://chromium-review.googlesource.com/c/1260053
Commit-Queue: Vadim Shtayura <vadimsh@chromium.org>
Reviewed-by: 's avatarAndrii Shyshkalov <tandrii@chromium.org>
parent 684313d6
...@@ -104,9 +104,27 @@ class CookiesAuthenticator(Authenticator): ...@@ -104,9 +104,27 @@ class CookiesAuthenticator(Authenticator):
Expected case for developer workstations. Expected case for developer workstations.
""" """
_EMPTY = object()
def __init__(self): def __init__(self):
self.netrc = self._get_netrc() # Credentials will be loaded lazily on first use. This ensures Authenticator
self.gitcookies = self._get_gitcookies() # get() can always construct an authenticator, even if something is broken.
# This allows 'creds-check' to proceed to actually checking creds later,
# rigorously (instead of blowing up with a cryptic error if they are wrong).
self._netrc = self._EMPTY
self._gitcookies = self._EMPTY
@property
def netrc(self):
if self._netrc is self._EMPTY:
self._netrc = self._get_netrc()
return self._netrc
@property
def gitcookies(self):
if self._gitcookies is self._EMPTY:
self._gitcookies = self._get_gitcookies()
return self._gitcookies
@classmethod @classmethod
def get_new_password_url(cls, host): def get_new_password_url(cls, host):
......
...@@ -2488,13 +2488,16 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase): ...@@ -2488,13 +2488,16 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase):
# For projects with unusual authentication schemes. # For projects with unusual authentication schemes.
# See http://crbug.com/603378. # See http://crbug.com/603378.
return return
# Lazy-loader to identify Gerrit and Git hosts.
if gerrit_util.GceAuthenticator.is_gce(): # Check presence of cookies only if using cookies-based auth method.
cookie_auth = gerrit_util.Authenticator.get()
if not isinstance(cookie_auth, gerrit_util.CookiesAuthenticator):
return return
# Lazy-loader to identify Gerrit and Git hosts.
self.GetCodereviewServer() self.GetCodereviewServer()
git_host = self._GetGitHost() git_host = self._GetGitHost()
assert self._gerrit_server and self._gerrit_host assert self._gerrit_server and self._gerrit_host
cookie_auth = gerrit_util.CookiesAuthenticator()
gerrit_auth = cookie_auth.get_auth_header(self._gerrit_host) gerrit_auth = cookie_auth.get_auth_header(self._gerrit_host)
git_auth = cookie_auth.get_auth_header(git_host) git_auth = cookie_auth.get_auth_header(git_host)
...@@ -2544,10 +2547,15 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase): ...@@ -2544,10 +2547,15 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase):
(self.GetIssueURL(), (self.GetIssueURL(),
'submitted' if status == 'MERGED' else 'abandoned')) 'submitted' if status == 'MERGED' else 'abandoned'))
if gerrit_util.GceAuthenticator.is_gce(): # TODO(vadimsh): For some reason the chunk of code below was skipped if
# 'is_gce' is True. I'm just refactoring it to be 'skip if not cookies'.
# Apparently this check is not very important? Otherwise get_auth_email
# could have been added to other implementations of Authenticator.
cookies_auth = gerrit_util.Authenticator.get()
if not isinstance(cookies_auth, gerrit_util.CookiesAuthenticator):
return return
cookies_user = gerrit_util.CookiesAuthenticator().get_auth_email(
self._GetGerritHost()) cookies_user = cookies_auth.get_auth_email(self._GetGerritHost())
if self.GetIssueOwner() == cookies_user: if self.GetIssueOwner() == cookies_user:
return return
logging.debug('change %s owner is %s, cookies user is %s', logging.debug('change %s owner is %s, cookies user is %s',
...@@ -4161,10 +4169,17 @@ def CMDcreds_check(parser, args): ...@@ -4161,10 +4169,17 @@ def CMDcreds_check(parser, args):
"""Checks credentials and suggests changes.""" """Checks credentials and suggests changes."""
_, _ = parser.parse_args(args) _, _ = parser.parse_args(args)
if gerrit_util.GceAuthenticator.is_gce(): # Code below checks .gitcookies. Abort if using something else.
authn = gerrit_util.Authenticator.get()
if not isinstance(authn, gerrit_util.CookiesAuthenticator):
if isinstance(authn, gerrit_util.GceAuthenticator):
DieWithError(
'This command is not designed for GCE, are you on a bot?\n'
'If you need to run this on GCE, export SKIP_GCE_AUTH_FOR_GIT=1 '
'in your env.')
DieWithError( DieWithError(
'This command is not designed for GCE, are you on a bot?\n' 'This command is not designed for bot environment. It checks '
'If you need to run this, export SKIP_GCE_AUTH_FOR_GIT=1 in your env.') '~/.gitcookies file not generally used on bots.')
checker = _GitCookiesChecker() checker = _GitCookiesChecker()
checker.ensure_configured_gitcookies() checker.ensure_configured_gitcookies()
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment