diff --git a/gerrit_util.py b/gerrit_util.py index be6bca6f4..7d4283558 100755 --- a/gerrit_util.py +++ b/gerrit_util.py @@ -107,7 +107,7 @@ class CookiesAuthenticator(Authenticator): if not parts[0].endswith('-review'): parts[0] += '-review' url = 'https://%s/new-password' % ('.'.join(parts)) - return 'You can (re)generate your credentails by visiting %s' % url + return 'You can (re)generate your credentials by visiting %s' % url @classmethod def get_netrc_path(cls): diff --git a/git_cl.py b/git_cl.py index aa2238ace..c3e564fd5 100755 --- a/git_cl.py +++ b/git_cl.py @@ -3566,6 +3566,11 @@ class _GitCookiesChecker(object): prefix = prefix[:-len('-review')] return prefix + '.' + self._GOOGLESOURCE + def _canonical_gerrit_googlesource_host(self, host): + git_host = self._canonical_git_googlesource_host(host) + prefix = git_host.split('.', 1)[0] + return prefix + '-review.' + self._GOOGLESOURCE + def has_generic_host(self): """Returns whether generic .googlesource.com has been configured. @@ -3624,6 +3629,63 @@ class _GitCookiesChecker(object): hosts.add(host) return hosts + @staticmethod + def print_hosts(hosts, extra_column_func=None): + hosts = sorted(hosts) + assert hosts + if extra_column_func is None: + extras = [''] * len(hosts) + else: + extras = [extra_column_func(host) for host in hosts] + tmpl = ' %%-%ds %%-%ds' % (max(map(len, hosts)), max(map(len, extras))) + for he in zip(hosts, extras): + print(tmpl % he) + print() + + def find_and_report_problems(self): + """Returns True if there was at least one problem, else False.""" + problems = [False] + def add_problem(): + if not problems[0]: + print('.gitcookies problem report:\n') + problems[0] = True + + if self.has_generic_host(): + add_problem() + print(' .googlesource.com record detected\n' + ' Chrome Infrastructure team recommends to list full host names ' + 'explicitly.\n') + + dups = self.get_duplicated_hosts() + if dups: + add_problem() + print(' The following hosts were defined twice:\n') + self.print_hosts(dups) + + partial = self.get_partially_configured_hosts() + if partial: + add_problem() + print(' Credentials should come in pairs for Git and Gerrit hosts. ' + 'These hosts are missing:') + self.print_hosts(partial) + + conflicting = self.get_conflicting_hosts() + if conflicting: + add_problem() + print(' The following Git hosts have differing credentials from their ' + 'Gerrit counterparts:\n') + self.print_hosts(conflicting, lambda host: '%s vs %s' % + tuple(self._get_git_gerrit_identity_pairs()[host])) + + wrong = self.get_hosts_with_wrong_identities() + if wrong: + add_problem() + print(' These hosts likely use wrong identity:\n') + self.print_hosts(wrong, lambda host: '%s but %s recommended' % + (self._get_git_gerrit_identity_pairs()[host][0], + self._EXPECTED_HOST_IDENTITY_DOMAINS[host])) + return problems[0] + def CMDcreds_check(parser, args): """Checks credentials and suggests changes.""" @@ -3635,11 +3697,13 @@ def CMDcreds_check(parser, args): checker = _GitCookiesChecker() checker.ensure_configured_gitcookies() - print('Your .netrc and .gitcookies have credentails for these hosts:') + print('Your .netrc and .gitcookies have credentials for these hosts:') checker.print_current_creds(include_netrc=True) - # TODO(tandrii): add report && autofixes. - return 0 + if not checker.find_and_report_problems(): + print('\nNo problems detected in your .gitcookies') + return 0 + return 1 @subcommand.usage('[repo root containing codereview.settings]') @@ -5068,7 +5132,7 @@ def CMDtry_results(parser, args): if not patchset: parser.error('Codereview doesn\'t know about issue %s. ' 'No access to issue or wrong issue number?\n' - 'Either upload first, or pass --patchset explicitely' % + 'Either upload first, or pass --patchset explicitly' % cl.GetIssue()) # TODO(tandrii): Checking local patchset against remote patchset is only diff --git a/tests/git_cl_creds_check_report.txt b/tests/git_cl_creds_check_report.txt new file mode 100644 index 000000000..87ce55b68 --- /dev/null +++ b/tests/git_cl_creds_check_report.txt @@ -0,0 +1,20 @@ +.gitcookies problem report: + + .googlesource.com record detected + Chrome Infrastructure team recommends to list full host names explicitly. + + The following hosts were defined twice: + + dup.googlesource.com + + Credentials should come in pairs for Git and Gerrit hosts. These hosts are missing: + partial.googlesource.com + + The following Git hosts have differing credentials from their Gerrit counterparts: + + conflict.googlesource.com git-example.google.com vs git-example.chromium.org + + These hosts likely use wrong identity: + + chrome-internal.googlesource.com git-example.chromium.org but google.com recommended + chromium.googlesource.com git-example.google.com but chromium.org recommended diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index 2aaa98487..2af31749a 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -447,10 +447,11 @@ class TestGitClBasic(unittest.TestCase): 'Cr-Branched-From: somehash-refs/heads/master@{#12}') -class GitCookiesCheckerTest(unittest.TestCase): +class GitCookiesCheckerTest(TestCase): def setUp(self): super(GitCookiesCheckerTest, self).setUp() self.c = git_cl._GitCookiesChecker() + self.c._all_hosts = [] def mock_hosts_creds(self, subhost_identity_pairs): def ensure_googlesource(h): @@ -495,6 +496,22 @@ class GitCookiesCheckerTest(unittest.TestCase): 'chrome-internal.googlesource.com']), self.c.get_hosts_with_wrong_identities()) + def test_report_no_problems(self): + self.test_analysis_nothing() + self.mock(sys, 'stdout', StringIO.StringIO()) + self.assertFalse(self.c.find_and_report_problems()) + self.assertEqual(sys.stdout.getvalue(), '') + + def test_report(self): + self.test_analysis() + self.mock(sys, 'stdout', StringIO.StringIO()) + self.assertTrue(self.c.find_and_report_problems()) + with open(os.path.join(os.path.dirname(__file__), + 'git_cl_creds_check_report.txt')) as f: + expected = f.read() + def by_line(text): + return [l.rstrip() for l in text.rstrip().splitlines()] + self.assertEqual(by_line(sys.stdout.getvalue()), by_line(expected)) class TestGitCl(TestCase): def setUp(self): @@ -1169,7 +1186,7 @@ class TestGitCl(TestCase): remote_ref='refs/heads/disabled') self.assertEqual(res, False) - # Validator is disabled by default, even if it's not explicitely in disabled + # Validator is disabled by default, even if it's not explicitly in disabled # refglobs. res = git_cl._is_git_numberer_enabled( remote_url='https://chromium.googlesource.com/chromium/src', @@ -2036,7 +2053,7 @@ class TestGitCl(TestCase): 'Credentials for the following hosts are required:\n' ' chromium-review.googlesource.com\n' 'These are read from ~/.gitcookies (or legacy ~/.netrc)\n' - 'You can (re)generate your credentails by visiting ' + 'You can (re)generate your credentials by visiting ' 'https://chromium-review.googlesource.com/new-password'],), ''),) self.assertIsNone(cl.EnsureAuthenticated(force=False)) @@ -3034,7 +3051,7 @@ class TestGitCl(TestCase): def test_creds_check_gitcookies_not_configured(self): self._common_creds_check_mocks() self.mock(git_cl._GitCookiesChecker, 'get_hosts_with_creds', - lambda _, include_netrc: []) + lambda _, include_netrc=False: []) self.calls = [ ((['git', 'config', '--global', 'http.cookiefile'],), CERR1), (('os.path.exists', '~/.netrc'), True), @@ -3054,7 +3071,7 @@ class TestGitCl(TestCase): def test_creds_check_gitcookies_configured_custom_broken(self): self._common_creds_check_mocks() self.mock(git_cl._GitCookiesChecker, 'get_hosts_with_creds', - lambda _, include_netrc: []) + lambda _, include_netrc=False: []) self.calls = [ ((['git', 'config', '--global', 'http.cookiefile'],), '/custom/.gitcookies'),