git cl creds-check: add reporting.

BUG=689543

Change-Id: I2c5dd9e1ef4a23ba805c8c057ad44c87cfb65a5b
Reviewed-on: https://chromium-review.googlesource.com/455782
Commit-Queue: Andrii Shyshkalov <tandrii@chromium.org>
Reviewed-by: Michael Achenbach <machenbach@chromium.org>

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

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

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

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'),