ninjalog_uploader: use goma_auth to detect googler

Bug: 1288639
Change-Id: I447e2f66603ffb8d68599dcf22023fd7857dc4fd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3400398Reviewed-by: Fumitoshi Ukai <ukai@google.com>
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Takuto Ikuta <tikuta@chromium.org>

PRESUBMIT.py

@@ -112,22 +112,36 @@ def CheckUnitTestsOnCommit(input_api, output_api):
       'recipes_test.py',
   ]
 
+  py3_only_tests = ['ninjalog_uploader_test.py']
+
   tests = input_api.canned_checks.GetUnitTestsInDirectory(
       input_api,
       output_api,
       'tests',
       files_to_check=test_to_run_list,
-      files_to_skip=tests_to_skip_list + py2_only_tests,
+      files_to_skip=tests_to_skip_list + py2_only_tests + py3_only_tests,
       run_on_python3=True)
 
   # TODO: once py3 compatbile, remove those tests
-  tests.extend(input_api.canned_checks.GetUnitTestsInDirectory(
-      input_api,
-      output_api,
-      'tests',
-      files_to_check=py2_only_tests,
-      files_to_skip=tests_to_skip_list,
-      run_on_python3=False))
+  tests.extend(
+      input_api.canned_checks.GetUnitTestsInDirectory(
+          input_api,
+          output_api,
+          'tests',
+          files_to_check=py2_only_tests,
+          files_to_skip=tests_to_skip_list,
+          run_on_python3=False))
+
+  # TODO: use this for all tests when py2 support is dropped.
+  tests.extend(
+      input_api.canned_checks.GetUnitTestsInDirectory(
+          input_api,
+          output_api,
+          'tests',
+          files_to_check=py3_only_tests,
+          files_to_skip=tests_to_skip_list,
+          run_on_python3=True,
+          run_on_python2=False))
 
   return input_api.RunTests(tests)
 
@@ -177,4 +191,3 @@ def CheckOwnersOnUpload(input_api, output_api):
 
 def CheckDoNotSubmitOnCommit(input_api, output_api):
   return input_api.canned_checks.CheckDoNotSubmit(input_api, output_api)
-

ninjalog.README.md

@@ -14,9 +14,7 @@ $ autoninja -C out/Release chrome
 autoninja uploads ninja's build log for Google employees but we don't collect
 logs from external contributors.
 
-We use [this page](https://chromium-build-stats-staging.appspot.com/should-upload)
-to decide whether an autoninja user is a Googler or not. Only people with access
-to Google's internal network can see 'Success'.
+We use `goma_auth info` to decide whether an autoninja user is a Googler or not.
 
 Before uploading logs, autoninja shows a message 10 times to warn users that we
 will collect build logs.

ninjalog_uploader.py

@@ -26,8 +26,6 @@ import subprocess
 import sys
 import time
 
-from third_party.six.moves import http_client
-from third_party.six.moves.urllib import error
 from third_party.six.moves.urllib import request
 
 # These build configs affect build performance.
@@ -39,13 +37,17 @@ ALLOWLISTED_CONFIGS = ('symbol_level', 'use_goma', 'is_debug',
                        'use_errorprone_java_compiler', 'incremental_install')
 
 
-def IsGoogler(server):
-  """Check whether this script run inside corp network."""
-  try:
-    resp = request.urlopen('https://' + server + '/should-upload')
-    return resp.read() == b'Success'
-  except (error.URLError, http_client.RemoteDisconnected):
+def IsGoogler():
+  """Check whether this user is Googler or not."""
+  p = subprocess.run('goma_auth info',
+                     capture_output=True,
+                     text=True,
+                     shell=True)
+  if p.returncode != 0:
     return False
+  l = p.stdout.splitlines()[0]
+  # |l| will be like 'Login as <user>@google.com' for googler using goma.
+  return l.startswith('Login as ') and l.endswith('@google.com')
 
 
 def ParseGNArgs(gn_args):
@@ -190,7 +192,7 @@ def main():
     # Disable logging.
     logging.disable(logging.CRITICAL)
 
-  if not IsGoogler(args.server):
+  if not IsGoogler():
     return 0
 
   ninjalog = args.ninjalog or GetNinjalog(args.cmdline)

ninjalog_uploader_wrapper.py

@@ -17,7 +17,7 @@ import subprocess2
 THIS_DIR = os.path.dirname(__file__)
 UPLOADER = os.path.join(THIS_DIR, 'ninjalog_uploader.py')
 CONFIG = os.path.join(THIS_DIR, 'ninjalog.cfg')
-VERSION = 2
+VERSION = 3
 
 
 def LoadConfig():
@@ -29,8 +29,7 @@ def LoadConfig():
         return config
 
   return {
-      'is-googler':
-      ninjalog_uploader.IsGoogler('chromium-build-stats.appspot.com'),
+      'is-googler': ninjalog_uploader.IsGoogler(),
       'countdown': 10,
       'version': VERSION,
   }
@@ -69,6 +68,8 @@ If you have questions about this, please send mail to infra-dev@chromium.org
 
 You can find a more detailed explanation in
 %s
+or
+https://chromium.googlesource.com/chromium/tools/depot_tools/+/main/ninjalog.README.md
 
 """ % (whitelisted, countdown, __file__, __file__,
        os.path.abspath(os.path.join(THIS_DIR, "ninjalog.README.md"))))

tests/ninjalog_uploader_test.py

@@ -7,6 +7,7 @@ import json
 import os
 import sys
 import unittest
+import unittest.mock
 
 ROOT_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
 sys.path.insert(0, ROOT_DIR)
@@ -15,6 +16,23 @@ import ninjalog_uploader
 
 
 class NinjalogUploaderTest(unittest.TestCase):
+  def test_IsGoogler(self):
+    with unittest.mock.patch('subprocess.run') as run_mock:
+      run_mock.return_value.returncode = 0
+      run_mock.return_value.stdout = ('Login as foo@google.com\n'
+                                      'goma is ready to use')
+      self.assertTrue(ninjalog_uploader.IsGoogler())
+
+    with unittest.mock.patch('subprocess.run') as run_mock:
+      run_mock.return_value.returncode = 1
+      run_mock.return_value.stdout = 'Not logged in\n'
+      self.assertFalse(ninjalog_uploader.IsGoogler())
+
+    with unittest.mock.patch('subprocess.run') as run_mock:
+      run_mock.return_value.returncode = 0
+      run_mock.return_value.stdout = 'Login as foo@example.com\n'
+      self.assertFalse(ninjalog_uploader.IsGoogler())
+
   def test_parse_gn_args(self):
     self.assertEqual(ninjalog_uploader.ParseGNArgs(json.dumps([])), {})