From b250ec16d39169c4f7df3f9ae40230934d0f4ddb Mon Sep 17 00:00:00 2001 From: Vadim Shtayura Date: Thu, 4 Oct 2018 00:21:08 +0000 Subject: [PATCH] [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 Reviewed-by: Andrii Shyshkalov --- gerrit_util.py | 22 ++++++++++++++++++++-- git_cl.py | 33 ++++++++++++++++++++++++--------- 2 files changed, 44 insertions(+), 11 deletions(-) diff --git a/gerrit_util.py b/gerrit_util.py index b81f7b5cc..e8e95610e 100644 --- a/gerrit_util.py +++ b/gerrit_util.py @@ -104,9 +104,27 @@ class CookiesAuthenticator(Authenticator): Expected case for developer workstations. """ + _EMPTY = object() + def __init__(self): - self.netrc = self._get_netrc() - self.gitcookies = self._get_gitcookies() + # Credentials will be loaded lazily on first use. This ensures Authenticator + # 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 def get_new_password_url(cls, host): diff --git a/git_cl.py b/git_cl.py index b4b8b65a0..902e4e2ef 100755 --- a/git_cl.py +++ b/git_cl.py @@ -2488,13 +2488,16 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase): # For projects with unusual authentication schemes. # See http://crbug.com/603378. 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 + + # Lazy-loader to identify Gerrit and Git hosts. self.GetCodereviewServer() git_host = self._GetGitHost() assert self._gerrit_server and self._gerrit_host - cookie_auth = gerrit_util.CookiesAuthenticator() gerrit_auth = cookie_auth.get_auth_header(self._gerrit_host) git_auth = cookie_auth.get_auth_header(git_host) @@ -2544,10 +2547,15 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase): (self.GetIssueURL(), '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 - cookies_user = gerrit_util.CookiesAuthenticator().get_auth_email( - self._GetGerritHost()) + + cookies_user = cookies_auth.get_auth_email(self._GetGerritHost()) if self.GetIssueOwner() == cookies_user: return logging.debug('change %s owner is %s, cookies user is %s', @@ -4161,10 +4169,17 @@ def CMDcreds_check(parser, args): """Checks credentials and suggests changes.""" _, _ = 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( - 'This command is not designed for GCE, are you on a bot?\n' - 'If you need to run this, export SKIP_GCE_AUTH_FOR_GIT=1 in your env.') + 'This command is not designed for bot environment. It checks ' + '~/.gitcookies file not generally used on bots.') checker = _GitCookiesChecker() checker.ensure_configured_gitcookies()