git cl try uses buildbucket via OAuth2

git_cl.py

 #!/usr/bin/env python
 # Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 # Copyright (C) 2008 Evan Martin <martine@danga.com>
 
 """A git-command for integrating reviews on Rietveld."""
 
 from distutils.version import LooseVersion
 from multiprocessing.pool import ThreadPool
 import base64
 import glob
+import httplib
 import json
 import logging
 import optparse
 import os
 import Queue
 import re
 import stat
 import sys
 import tempfile
 import textwrap
+import threading
+import time
 import urllib2
 import urlparse
 import webbrowser
 import zlib
 
 try:
   import readline  # pylint: disable=F0401,W0611
 except ImportError:
   pass
 
 
 from third_party import colorama
+from third_party import httplib2
 from third_party import upload
+from third_party.google_api_python_client import apiclient

[Vadim Sh., 2015/04/10 02:24:29]

it is not vendored properly:

Traceback (most recent call last):
  File "/Users/vadimsh/depot_tools/git_cl.py", line 36, in <module>
    from third_party import upload
  File "/Users/vadimsh/depot_tools/third_party/upload.py", line 75, in <module>
    import auth
  File "/Users/vadimsh/depot_tools/auth.py", line 26, in <module>
    from third_party.google_api_python_client import apiclient
  File
"/Users/vadimsh/depot_tools/third_party/google_api_python_client/apiclient/__init__.py",
line 3, in <module>
    import googleapiclient
ImportError: No module named googleapiclient

I suspect you have it installed in your global site-packages and it is imported
from there.

I personally think apiclient.discovery is not worth the trouble of adding
apiclient dependency (e.g. it is simpler to use httplib2 directly to send HTTP
requests to JSON endpoints). But as you prefer.

[nodir, 2015/04/13 16:04:53]

"import apiclient" should work, we use it in other places. If it doesn't
PYTHONPATH needs to be updated

[sheyang, 2015/04/14 01:30:22]

Per discussion with Nodir@ I added the directory to syspath to fix it.

 import breakpad  # pylint: disable=W0611
 import clang_format
 import dart_format
 import fix_encoding
 import gclient_utils
 import git_common
+import oauth2

[Vadim Sh., 2015/04/10 02:24:29]

this file has been replaced by auth.py in
https://codereview.chromium.org/1074673002/ (will commit it tomorrow).

auth.py is used by "git cl upload" and all other depot_tools script. See comment
below for how to use it for accessing buildbucket.

[sheyang, 2015/04/14 01:30:22]

Done.

 import owners
 import owners_finder
 import presubmit_support
 import rietveld
 import scm
 import subcommand
 import subprocess2
 import watchlists
 
 __version__ = '1.0'
 
 DEFAULT_SERVER = 'https://codereview.appspot.com'
 POSTUPSTREAM_HOOK_PATTERN = '.git/hooks/post-cl-%s'
 DESCRIPTION_BACKUP_FILE = '~/.git_cl_description_backup'
 GIT_INSTRUCTIONS_URL = 'http://code.google.com/p/chromium/wiki/UsingGit'
 CHANGE_ID = 'Change-Id:'
 REFS_THAT_ALIAS_TO_OTHER_REFS = {
     'refs/remotes/origin/lkgr': 'refs/remotes/origin/master',
     'refs/remotes/origin/lkcr': 'refs/remotes/origin/master',
 }
 
+# Buildbucket-related constants
+DISCOVERY_URL = (
+    'https://cr-buildbucket.appspot.com/_ah/api/discovery/v1/apis/'
+    '{api}/{apiVersion}/rest')
+DEFAULT_SCOPES = 'email'

[Vadim Sh., 2015/04/10 02:24:29]

not used

[sheyang, 2015/04/14 01:30:22]

Removed.

+BUILDSET_STR = 'patch/rietveld/{hostname}/{issue}/{patch}'
+
 # Valid extensions for files we want to lint.
 DEFAULT_LINT_REGEX = r"(.*\.cpp|.*\.cc|.*\.h)"
 DEFAULT_LINT_IGNORE_REGEX = r"$^"
 
 # Shortcut since it quickly becomes redundant.
 Fore = colorama.Fore
 
 # Initialized in main()
 settings = None
 
@@ skipping 134 matching lines @@
   if dirty:
     print 'Cannot %s with a dirty tree. You must commit locally first.' % cmd
     print 'Uncommitted files: (git diff-index --name-status HEAD)'
     print dirty[:4096]
     if len(dirty) > 4096:
       print '... (run "git diff-index --name-status HEAD" to see full output).'
     return True
   return False
 
 
+def _prefix_master(master):
+  prefix = 'master.'
+  if master.startswith(prefix):
+    return master
+  else:
+    return '%s%s' % (prefix, master)
+
+
+def _get_buildbucket(credentials):

[Vadim Sh., 2015/04/10 02:24:29]

with auth.py it would be:

def _get_buildbucket(rietveld_url, auth_config):
  authenticator = auth.get_authenticator_for_host(rietveld_url, auth_config)
  http = httplib2.Http()
  authenticator.authorize(http)
  return ...

auth_config is already extracted from command line options CMDtry. rietveld_url
is changelist.GetRietveldServer().

[sheyang, 2015/04/14 01:30:22]

Done.

+  http = httplib2.Http()
+  http = credentials.authorize(http)
+  return apiclient.discovery.build(
+      'buildbucket', 'v1',
+      http=http,
+      discoveryServiceUrl=DISCOVERY_URL,
+  )
+
+
+def trigger_distributed_try_jobs(
+    credentials, changelist, options, masters, category):
+  buildbucket = _get_buildbucket(credentials)
+  creds_props = json.loads(credentials.to_json())
+  requester = creds_props['id_token']['email']

[Vadim Sh., 2015/04/10 02:24:29]

this is currently not exposed via auth.py, though I can add it. But I think it
is a wrong approach security-wise, see comment below.

[sheyang, 2015/04/14 01:30:22]

This field will be populated from oauth2 credential. Replaced it with user_agent
and the value is 'git+cl_try'.

+  issue_props = changelist.GetIssueProperties()
+  rietveld_host = urlparse.urlparse(changelist.GetRietveldServer()).hostname
+  issue = changelist.GetIssue()
+  patchset = changelist.GetMostRecentPatchset()
+  buildset = BUILDSET_STR.format(
+      hostname=rietveld_host,
+      issue=str(issue),
+      patch=str(patchset))
+  print 'Tried jobs on:'
+  for (master, builders_and_tests) in masters.iteritems():
+    print 'Master: %s' % master
+    bucket = _prefix_master(master)
+    for builder, tests in builders_and_tests.iteritems():
+      req = buildbucket.put(body={
+          'bucket': bucket,
+          'parameters_json': json.dumps({
+              'builder_name': builder,
+              'changes':[
+                  {'author': {'email': issue_props['owner_email']}},
+              ],
+              'properties': {
+                  'category': category,
+                  'clobber': options.clobber,
+                  'issue': issue,
+                  'master': master,
+                  'patch_project': issue_props['project'],
+                  'patch_storage': 'rietveld',
+                  'patchset': patchset,
+                  'reason': options.name,
+                  'requester': requester,

[Vadim Sh., 2015/04/10 02:24:29]

the server should derive it from oauth credentials itself, or at least verify if
corresponds to email associated with oauth token. Otherwise it is trivial to
trick server and request jobs on somebody else's behalf. +nodir to answer this
question.

[nodir, 2015/04/10 02:31:29]

buidbucket does not read this property. It is interpreted by a recipe, that does
not verify identity. Buildbot trusts buildbucket that its buckets are secure.
BuildBucket trusts that authorized users won't put garbage to properties.

requester is not always == created_by because CQ schedules builds on behalf of
its users, so "requester" "enables" impersonation (in insecure way I guess)

[nodir, 2015/04/13 16:04:53]

This part was wrong because CQ sets requester to "commit-bot@chromium.org", so
requester should be read from buildbucket.build.created_by property. In order to
not break the existing contract between buildbot and recipes, I think
buildbot-buildbucket integration should copy created_by to requester property.

[sheyang, 2015/04/14 01:30:22]

Acknowledged.

+                  'revision': options.revision,
+                  'rietveld': changelist.GetRietveldServer(),
+                  'testfilter': tests,
+              },
+          }),
+          'tags': ['buildset:%s' % buildset,
+                   'master:%s' % master,
+                   'builder:%s' % builder,
+                   'requester:%s' % requester]

[nodir, 2015/04/13 16:04:53]

requester tag is not the user, but a tool. Maybe we should rename it to
"requester_tool" 
https://cr-buildbucket.appspot.com/#docs/conventions
CQ specifies "cq" here. git-cl-try should probably specify "git-cl-try".
Rietveld will specify "rietveld"

[sheyang, 2015/04/14 01:30:22]

Use 'user_agent' instead.

+      })
+      wait = 1
+      try_count = 3
+      while try_count > 0:
+        try:
+          try_count -= 1
+          response = req.execute()
+          if response.get('error'):
+            msg = 'Error in response. Reason: %s. Message: %s.' % (
+                response['error'].get('reason', ''),
+                response['error'].get('message', ''))
+            raise BuildbucketResponseException(msg)
+          break
+        except apiclient.errors.HttpError as ex:
+          status = ex.resp.status if ex.resp else None
+          if status >= 500:
+            logging.debug('Transient errors when triggering tryjobs. '
+                          'Will retry in %d seconds.', wait)
+            time.sleep(wait)
+            wait *= 2
+            if try_count <= 0:
+              raise
+          else:
+            raise
+      print '  %s: %s' % (builder, tests)
+
+
 def MatchSvnGlob(url, base_url, glob_spec, allow_wildcards):
   """Return the corresponding git ref if |base_url| together with |glob_spec|
   matches the full |url|.
 
   If |allow_wildcards| is true, |glob_spec| can contain wildcards (see below).
   """
   fetch_suburl, as_ref = glob_spec.split(':')
   if allow_wildcards:
     glob_match = re.match('(.+/)?(\*|{[^/]*})(/.+)?', fetch_suburl)
     if glob_match:
@@ skipping 47 matching lines @@
   try:
     stdout = sys.stdout.fileno()
   except AttributeError:
     stdout = None
   return subprocess2.call(
       ['git',
        'diff', '--no-ext-diff', '--stat'] + similarity_options + args,
       stdout=stdout, env=env)
 
 
+class BuildbucketResponseException(Exception):
+  pass
+
+
 class Settings(object):
   def __init__(self):
     self.default_server = None
     self.cc = None
     self.root = None
     self.is_git_svn = None
     self.svn_branch = None
     self.tree_status_url = None
     self.viewvc_url = None
     self.updated = False
@@ skipping 2434 matching lines @@
       "-c", "--clobber", action="store_true", default=False,
       help="Force a clobber before building; e.g. don't do an "
            "incremental build")
   group.add_option(
       "--project",
       help="Override which project to use. Projects are defined "
            "server-side to define what default bot set to use")
   group.add_option(
       "-n", "--name", help="Try job name; default to current branch name")
   parser.add_option_group(group)
+  oauth2.add_oauth2_options(parser)
   options, args = parser.parse_args(args)
 
   if args:
     parser.error('Unknown arguments: %s' % args)
 
   cl = Changelist()
   if not cl.GetIssue():
     parser.error('Need to upload first')
 
   props = cl.GetIssueProperties()
@@ skipping 75 matching lines @@
           'Bot list: %s' % builders)
       return 1
 
   patchset = cl.GetMostRecentPatchset()
   if patchset and patchset != cl.GetPatchset():
     print(
         '\nWARNING Mismatch between local config and server. Did a previous '
         'upload fail?\ngit-cl try always uses latest patchset from rietveld. '
         'Continuing using\npatchset %s.\n' % patchset)
   try:
-    cl.RpcServer().trigger_distributed_try_jobs(
-        cl.GetIssue(), patchset, options.name, options.clobber,
-        options.revision, masters)
-  except urllib2.HTTPError, e:
-    if e.code == 404:
-      print('404 from rietveld; '
-            'did you mean to use "git try" instead of "git cl try"?')
+    creds = oauth2.get_oauth2_creds(
+        options,
+        urlparse.urlparse(cl.GetRietveldServer()).hostname)
+    if not creds:
+      print 'Failed to fetch credentials. Aborting...'
       return 1
-  print('Tried jobs on:')
-
-  for (master, builders) in masters.iteritems():
-    if master:
-      print 'Master: %s' % master
-    length = max(len(builder) for builder in builders)
-    for builder in sorted(builders):
-      print '  %*s: %s' % (length, builder, ','.join(builders[builder]))
+    trigger_distributed_try_jobs(creds, cl, options, masters, 'git cl try')
+  except apiclient.errors.HttpError as ex:
+    status = ex.resp.status if ex.resp else None
+    if status == httplib.FORBIDDEN:
+      print 'ERROR: Access denied. Please verify you have tryjob access.'
+    else:
+      print 'ERROR: Tryjob request failed: %s.' % ex
+    return 1
+  except BuildbucketResponseException as ex:
+    print ex
+    return 1
+  except Exception as e:
+    print 'Unexpected error when trying to trigger tryjobs: %s.' % e
+    return 1
   return 0
 
 
 @subcommand.usage('[new upstream branch]')
 def CMDupstream(parser, args):
   """Prints or sets the name of the upstream branch, if any."""
   _, args = parser.parse_args(args)
   if len(args) > 1:
     parser.error('Unrecognized args: %s' % ' '.join(args))
 
@@ skipping 301 matching lines @@
 if __name__ == '__main__':
   # These affect sys.stdout so do it outside of main() to simplify mocks in
   # unit testing.
   fix_encoding.fix_encoding()
   colorama.init()
   try:
     sys.exit(main(sys.argv[1:]))
   except KeyboardInterrupt:
     sys.stderr.write('interrupted\n')
     sys.exit(1)