autoland: add request parsing for patch based requests (bug 1368516) r?smacleod draft
authorbyron jones <glob@mozilla.com>
Tue, 25 Jul 2017 15:34:14 +0800
changeset 11681 567825c58a84c178b728bff0922968408085f4e0
parent 11680 fee2d454c1d6ee6d0380ab594d94afca826e7a38
child 11682 602f2d837a3b5052304b475eea62a2de7d6cf976
push id1790
push userbjones@mozilla.com
push dateTue, 19 Sep 2017 04:17:41 +0000
reviewerssmacleod
bugs1368516
autoland: add request parsing for patch based requests (bug 1368516) r?smacleod Add a patch_url field to requests, and use that to detect the type of request (from repo vs patch). This also tightens request verification to ensure conflicting fields cannot be provided, and rejects requests with extraneous fields as the request is stored verbatim in the database. Handling of patch based requests is not yet fully implemented. MozReview-Commit-ID: 6MsznY9oX3u
autoland/autoland/autoland_rest.py
autoland/tests/test-autoland-closed-tree.t
autoland/tests/test-autoland-merge.t
autoland/tests/test-post-autoland-job.t
testing/vcttesting/autoland_mach_commands.py
--- a/autoland/autoland/autoland_rest.py
+++ b/autoland/autoland/autoland_rest.py
@@ -69,52 +69,114 @@ def check_pingback_url(pingback_url):
         if url.hostname == allowed_host:
             return True
 
     return False
 
 
 def validate_request(request):
     if request.json is None:
-        raise ValueError('Bad request: missing json')
+        raise ValueError('missing json')
+    request_json = request.json
+
+    required = {'ldap_username', 'tree', 'rev', 'pingback_url', 'destination'}
+    optional = set()
+
+    is_try = 'trysyntax' in request_json
+    is_patch = 'patch_urls' in request_json
 
-    for field in ['ldap_username', 'tree', 'rev', 'destination',
-                  'pingback_url']:
-        if field not in request.json:
-            raise ValueError('missing json field: %s' % field)
+    if (not is_patch) and not ('trysyntax' in request_json or
+                               'commit_descriptions' in request_json):
+        raise ValueError('one of trysyntax or commit_descriptions must be '
+                         'specified')
+
+    if not is_try and not is_patch:
+        # Repo transplant.
+        required.add('commit_descriptions')
+        optional.add('push_bookmark')
+
+    elif not is_try and is_patch:
+        # Patch transplant.
+        required.add('patch_urls')
+        optional.add('push_bookmark')
 
-    if not check_pingback_url(request.json['pingback_url']):
-        raise ValueError('bad pingback_url')
+    elif is_try and not is_patch:
+        # Repo try.
+        required.add('trysyntax')
+
+    elif is_try and is_patch:
+        # Patch try.
+        raise ValueError('trysyntax is not supported with patch_urls')
+
+    request_fields = set(request_json.keys())
 
-    if not (request.json.get('trysyntax') or
-            request.json.get('commit_descriptions')):
-        raise ValueError('one of trysyntax or commit_descriptions must be '
-                         'specified.')
+    missing = required - request_fields
+    if missing:
+        raise ValueError('missing required field%s: %s' % (
+            '' if len(missing) == 1 else 's',
+            ', '.join(sorted(missing))))
+
+    extra = request_fields - (required | optional)
+    if extra:
+        raise ValueError('unexpected field%s: %s' % (
+            '' if len(extra) == 1 else 's',
+            ', '.join(sorted(extra))))
+
+    if not check_pingback_url(request_json['pingback_url']):
+        raise ValueError('bad pingback_url')
 
 
 @app.route('/autoland', methods=['POST'])
 def autoland():
     """
     Autoland a patch from one tree to another.
 
-    Example request json:
+    Example repository based landing request:
+    (All fields are required except for push_bookmark)
+
+    {
+      "ldap_username": "cthulhu@mozilla.org",
+      "tree": "mozilla-central",
+      "rev": "9cc25f7ac50a",
+      "destination": "gecko",
+      "commit_descriptions": {"9cc25f7ac50a": "bug 1 - did stuff r=gps"},
+      "pingback_url": "http://localhost/",
+      "push_bookmark": "@"
+    }
+
+    Example repository based try request:
+    (All fields are required)
 
     {
       "ldap_username": "cthulhu@mozilla.org",
       "tree": "mozilla-central",
       "rev": "9cc25f7ac50a",
       "destination": "try",
-      "trysyntax": "try: -b o -p linux -u mochitest-1 -t none",
-      "push_bookmark": "@",
-      "commit_descriptions": {"9cc25f7ac50a": "bug 1 - did stuff r=gps"},
-      "pingback_url": "http://localhost/"
+      "pingback_url": "http://localhost/",
+      "trysyntax": "try: -b o -p linux -u mochitest-1 -t none"
     }
 
-    The trysyntax, push_bookmark and commit_descriptions fields are
-    optional, but one of trysyntax or commit_descriptions must be specified.
+    Example patch based landing request:
+
+    {
+      "ldap_username": "cthulhu@mozilla.org",
+      "tree": "mozilla-central",
+      "rev": "1235",
+      "patch_urls": ["https://example.com/123456789.patch"],
+      "destination": "gecko",
+      "pingback_url": "http://localhost/",
+      "push_bookmark": "@"
+    }
+
+    Patch based try requests are not supported.
+
+    Differences between repository and patch based requests:
+      "rev" changes from sha of source tree to unique ID
+      "patch_urls" added with URLs to the patch files
+      "commit_descriptions" removed
 
     Returns an id which can be used to get the status of the autoland
     request.
 
     """
 
     auth = request.authorization
     auth_response = {'WWW-Authenticate': 'Basic realm="Login Required"'}
@@ -123,18 +185,22 @@ def autoland():
     if not check_auth(auth.username, auth.password):
         logging.warn('Failed authentication for "%s" from %s' % (
             auth.username, request.remote_addr))
         return Response('Login required', 401, auth_response)
 
     try:
         validate_request(request)
     except ValueError as e:
+        app.logger.warn('Bad Request from %s: %s' % (request.remote_addr, e))
         return make_response(jsonify({'error': 'Bad request: %s' % e}), 400)
 
+    if 'patch_urls' in request.json:
+        raise NotImplementedError('patch based landings not implemented')
+
     dbconn = get_dbconn()
     cursor = dbconn.cursor()
 
     query = """
         SELECT created, request->>'ldap_username'
           FROM Transplant
          WHERE landed IS NULL
                AND request->>'rev' = %s
--- a/autoland/tests/test-autoland-closed-tree.t
+++ b/autoland/tests/test-autoland-closed-tree.t
@@ -52,12 +52,12 @@ Post a job
   (200, u'{\n  "request_id": 1\n}')
   $ ottoland autoland-job-status $AUTOLAND_URL 1 --poll
   timed out
 
 Open the tree
 
   $ treestatus set-status $TREESTATUS_URL try open
   $ ottoland autoland-job-status $AUTOLAND_URL 1 --poll
-  (200, u'{\n  "commit_descriptions": "", \n  "destination": "try", \n  "error_msg": "", \n  "landed": true, \n  "ldap_username": "autolanduser@example.com", \n  "push_bookmark": "", \n  "result": "*", \n  "rev": "f363e17530eb", \n  "tree": "test-repo", \n  "trysyntax": "stuff"\n}') (glob)
+  (200, u'{\n  "destination": "try", \n  "error_msg": "", \n  "landed": true, \n  "ldap_username": "autolanduser@example.com", \n  "result": "*", \n  "rev": "f363e17530eb", \n  "tree": "test-repo", \n  "trysyntax": "stuff"\n}') (glob)
 
   $ mozreview stop
   stopped 9 containers
--- a/autoland/tests/test-autoland-merge.t
+++ b/autoland/tests/test-autoland-merge.t
@@ -39,17 +39,17 @@ Create a commit to test on Try
   (visit review url to publish these review requests so others can see them)
 
 Post a job
 
   $ REV=`hg log -r . --template "{node|short}"`
   $ ottoland post-autoland-job $AUTOLAND_URL test-repo $REV inbound http://localhost:9898 --commit-descriptions "{\"$REV\": \"Bug 1 - some stuff; r=cthulhu\"}"
   (200, u'{\n  "request_id": 1\n}')
   $ ottoland autoland-job-status $AUTOLAND_URL 1 --poll
-  (200, u'{\n  "commit_descriptions": {\n    "e2507be7827c": "Bug 1 - some stuff; r=cthulhu"\n  }, \n  "destination": "inbound", \n  "error_msg": "", \n  "landed": true, \n  "ldap_username": "autolanduser@example.com", \n  "push_bookmark": "", \n  "result": "3bce87fd55d0", \n  "rev": "e2507be7827c", \n  "tree": "test-repo", \n  "trysyntax": ""\n}')
+  (200, u'{\n  "commit_descriptions": {\n    "e2507be7827c": "Bug 1 - some stuff; r=cthulhu"\n  }, \n  "destination": "inbound", \n  "error_msg": "", \n  "landed": true, \n  "ldap_username": "autolanduser@example.com", \n  "result": "3bce87fd55d0", \n  "rev": "e2507be7827c", \n  "tree": "test-repo"\n}')
   $ mozreview exec autoland hg log /repos/inbound-test-repo/ --template '{rev}:{desc\|firstline}:{phase}\\n'
   0:Bug 1 - some stuff; r=cthulhu:public
 
 Post a job with a bad merge
 
   $ mozreview exec autoland "bash -c cd /repos/inbound-test-repo/ && echo foo2 > foo"
   $ mozreview exec autoland "bash -c cd /repos/inbound-test-repo/ && hg commit -m \"trouble\""
   $ echo foo3 > foo
@@ -80,15 +80,15 @@ Post a job with a bad merge
   review url: http://$DOCKER_HOSTNAME:$HGPORT1/r/1 (draft)
   
   (review requests lack reviewers; visit review url to assign reviewers)
   (visit review url to publish these review requests so others can see them)
   $ REV=`hg log -r . --template "{node|short}"`
   $ ottoland post-autoland-job $AUTOLAND_URL test-repo $REV inbound http://localhost:9898 --commit-descriptions "{\"$REV\": \"Bug 1 - more stuff; r=cthulhu\"}"
   (200, u'{\n  "request_id": 2\n}')
   $ ottoland autoland-job-status $AUTOLAND_URL 2 --poll
-  (200, u'{\n  "commit_descriptions": {\n    "5d8686a5858e": "Bug 1 - more stuff; r=cthulhu"\n  }, \n  "destination": "inbound", \n  "error_msg": "We\'re sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.\\n\\nhg error in cmd: hg rebase -s 1fa31b8c94db -d 3bce87fd55d0: rebasing 4:1fa31b8c94db \\"Bug 1 - more stuff; r=cthulhu\\" (tip)\\nmerging foo\\nwarning: conflicts while merging foo! (edit, then use \'hg resolve --mark\')\\nunresolved conflicts (see hg resolve, then hg rebase --continue)\\n", \n  "landed": false, \n  "ldap_username": "autolanduser@example.com", \n  "push_bookmark": "", \n  "result": "", \n  "rev": "5d8686a5858e", \n  "tree": "test-repo", \n  "trysyntax": ""\n}')
+  (200, u'{\n  "commit_descriptions": {\n    "5d8686a5858e": "Bug 1 - more stuff; r=cthulhu"\n  }, \n  "destination": "inbound", \n  "error_msg": "We\'re sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.\\n\\nhg error in cmd: hg rebase -s 1fa31b8c94db -d 3bce87fd55d0: rebasing 4:1fa31b8c94db \\"Bug 1 - more stuff; r=cthulhu\\" (tip)\\nmerging foo\\nwarning: conflicts while merging foo! (edit, then use \'hg resolve --mark\')\\nunresolved conflicts (see hg resolve, then hg rebase --continue)\\n", \n  "landed": false, \n  "ldap_username": "autolanduser@example.com", \n  "result": "", \n  "rev": "5d8686a5858e", \n  "tree": "test-repo"\n}')
 
   $ mozreview exec autoland hg log /repos/inbound-test-repo/ --template '{rev}:{desc\|firstline}:{phase}\\n'
   0:Bug 1 - some stuff; r=cthulhu:public
 
   $ mozreview stop
   stopped 9 containers
--- a/autoland/tests/test-post-autoland-job.t
+++ b/autoland/tests/test-post-autoland-job.t
@@ -49,41 +49,41 @@ Posting a job with bad credentials shoul
   $ ottoland post-autoland-job $AUTOLAND_URL test-repo `hg log -r . --template "{node|short}"` try http://localhost:9898 --user blah --password blah
   (401, u'Login required')
   $ mozreview exec autoland tail -n1 /var/log/apache2/error.log
   * WARNING:root:Failed authentication for "blah" from * (glob)
 
 Posting a job with without both trysyntax and commit_descriptions should fail
 
   $ ottoland post-autoland-job $AUTOLAND_URL test-repo 42 try http://localhost:9898
-  (400, u'{\n  "error": "Bad request: one of trysyntax or commit_descriptions must be specified."\n}')
+  (400, u'{\n  "error": "Bad request: one of trysyntax or commit_descriptions must be specified"\n}')
 
 Posting a job with an unknown revision should fail
 
   $ ottoland post-autoland-job $AUTOLAND_URL test-repo 42 try http://localhost:9898 --commit-descriptions "{\"42\": \"bad revision\"}"
   (200, u'{\n  "request_id": 1\n}')
   $ ottoland autoland-job-status $AUTOLAND_URL 1 --poll
-  (200, u'{\n  "commit_descriptions": {\n    "42": "bad revision"\n  }, \n  "destination": "try", \n  "error_msg": "hg error in cmd: hg pull test-repo -r 42: pulling from http://hgrb/test-repo\\nabort: unknown revision \'42\'!\\n", \n  "landed": false, \n  "ldap_username": "autolanduser@example.com", \n  "push_bookmark": "", \n  "result": "", \n  "rev": "42", \n  "tree": "test-repo", \n  "trysyntax": ""\n}')
+  (200, u'{\n  "commit_descriptions": {\n    "42": "bad revision"\n  }, \n  "destination": "try", \n  "error_msg": "hg error in cmd: hg pull test-repo -r 42: pulling from http://hgrb/test-repo\\nabort: unknown revision \'42\'!\\n", \n  "landed": false, \n  "ldap_username": "autolanduser@example.com", \n  "result": "", \n  "rev": "42", \n  "tree": "test-repo"\n}')
 
 Post a job
 
   $ REV=`hg log -r . --template "{node|short}"`
   $ ottoland post-autoland-job $AUTOLAND_URL test-repo $REV inbound http://localhost:9898 --commit-descriptions "{\"$REV\": \"Bug 1 - some stuff; r=cthulhu\"}"
   (200, u'{\n  "request_id": 2\n}')
   $ ottoland autoland-job-status $AUTOLAND_URL 2 --poll
-  (200, u'{\n  "commit_descriptions": {\n    "e2507be7827c": "Bug 1 - some stuff; r=cthulhu"\n  }, \n  "destination": "inbound", \n  "error_msg": "", \n  "landed": true, \n  "ldap_username": "autolanduser@example.com", \n  "push_bookmark": "", \n  "result": "3bce87fd55d0", \n  "rev": "e2507be7827c", \n  "tree": "test-repo", \n  "trysyntax": ""\n}')
+  (200, u'{\n  "commit_descriptions": {\n    "e2507be7827c": "Bug 1 - some stuff; r=cthulhu"\n  }, \n  "destination": "inbound", \n  "error_msg": "", \n  "landed": true, \n  "ldap_username": "autolanduser@example.com", \n  "result": "3bce87fd55d0", \n  "rev": "e2507be7827c", \n  "tree": "test-repo"\n}')
   $ mozreview exec autoland hg log /repos/inbound-test-repo/ --template '{rev}:{desc\|firstline}:{phase}\\n'
   0:Bug 1 - some stuff; r=cthulhu:public
 
 Post a job with try syntax
 
-  $ ottoland post-autoland-job $AUTOLAND_URL test-repo `hg log -r . --template "{node|short}"` try http://localhost:9898 --trysyntax "stuff"
+  $ ottoland post-autoland-job $AUTOLAND_URL test-repo $REV try http://localhost:9898 --trysyntax "stuff"
   (200, u'{\n  "request_id": 3\n}')
   $ ottoland autoland-job-status $AUTOLAND_URL 3 --poll
-  (200, u'{\n  "commit_descriptions": "", \n  "destination": "try", \n  "error_msg": "", \n  "landed": true, \n  "ldap_username": "autolanduser@example.com", \n  "push_bookmark": "", \n  "result": "*", \n  "rev": "e2507be7827c", \n  "tree": "test-repo", \n  "trysyntax": "stuff"\n}') (glob)
+  (200, u'{\n  "destination": "try", \n  "error_msg": "", \n  "landed": true, \n  "ldap_username": "autolanduser@example.com", \n  "result": "*", \n  "rev": "e2507be7827c", \n  "tree": "test-repo", \n  "trysyntax": "stuff"\n}') (glob)
   $ mozreview exec autoland hg log /repos/try/ --template '{rev}:{desc\|firstline}:{phase}\\n'
   2:try: stuff:draft
   1:Bug 1 - some stuff; r?cthulhu:draft
   0:root commit:public
 
 Post a job using a bookmark
 
   $ echo foo2 > foo
@@ -109,17 +109,17 @@ Post a job using a bookmark
   review id:  bz://1/mynick
   review url: http://$DOCKER_HOSTNAME:$HGPORT1/r/1 (draft)
   (visit review url to publish these review requests so others can see them)
 
   $ REV=`hg log -r . --template "{node|short}"`
   $ ottoland post-autoland-job $AUTOLAND_URL test-repo $REV inbound http://localhost:9898 --push-bookmark "bookmark" --commit-descriptions "{\"$REV\": \"Bug 1 - more goodness; r=cthulhu\"}"
   (200, u'{\n  "request_id": 4\n}')
   $ ottoland autoland-job-status $AUTOLAND_URL 4 --poll
-  (200, u'{\n  "commit_descriptions": {\n    "373b6ff60965": "Bug 1 - more goodness; r=cthulhu"\n  }, \n  "destination": "inbound", \n  "error_msg": "", \n  "landed": true, \n  "ldap_username": "autolanduser@example.com", \n  "push_bookmark": "bookmark", \n  "result": "aba7cf08f2e4", \n  "rev": "373b6ff60965", \n  "tree": "test-repo", \n  "trysyntax": ""\n}')
+  (200, u'{\n  "commit_descriptions": {\n    "373b6ff60965": "Bug 1 - more goodness; r=cthulhu"\n  }, \n  "destination": "inbound", \n  "error_msg": "", \n  "landed": true, \n  "ldap_username": "autolanduser@example.com", \n  "push_bookmark": "bookmark", \n  "result": "aba7cf08f2e4", \n  "rev": "373b6ff60965", \n  "tree": "test-repo"\n}')
   $ mozreview exec autoland hg log /repos/inbound-test-repo/ --template '{rev}:{desc\|firstline}:{phase}\\n'
   1:Bug 1 - more goodness; r=cthulhu:public
   0:Bug 1 - some stuff; r=cthulhu:public
 
 Post a job with unicode commit descriptions to be rewritten
 
   $ echo foo3 > foo
   $ hg commit --encoding utf-8 -m 'Bug 1 - こんにちは; r?cthulhu'
@@ -147,17 +147,17 @@ Post a job with unicode commit descripti
   
   review id:  bz://1/mynick
   review url: http://$DOCKER_HOSTNAME:$HGPORT1/r/1 (draft)
   (visit review url to publish these review requests so others can see them)
   $ REV=`hg log -r . --template "{node|short}"`
   $ ottoland post-autoland-job $AUTOLAND_URL test-repo $REV inbound http://localhost:9898 --commit-descriptions "{\"$REV\": \"Bug 1 - \\u3053\\u3093\\u306b\\u3061\\u306f; r=cthulhu\"}"
   (200, u'{\n  "request_id": 5\n}')
   $ ottoland autoland-job-status $AUTOLAND_URL 5 --poll
-  (200, u'{\n  "commit_descriptions": {\n    "e7f4a0f07be3": "Bug 1 - \\u3053\\u3093\\u306b\\u3061\\u306f; r=cthulhu"\n  }, \n  "destination": "inbound", \n  "error_msg": "", \n  "landed": true, \n  "ldap_username": "autolanduser@example.com", \n  "push_bookmark": "", \n  "result": "8ec6d9d32147", \n  "rev": "e7f4a0f07be3", \n  "tree": "test-repo", \n  "trysyntax": ""\n}')
+  (200, u'{\n  "commit_descriptions": {\n    "e7f4a0f07be3": "Bug 1 - \\u3053\\u3093\\u306b\\u3061\\u306f; r=cthulhu"\n  }, \n  "destination": "inbound", \n  "error_msg": "", \n  "landed": true, \n  "ldap_username": "autolanduser@example.com", \n  "result": "8ec6d9d32147", \n  "rev": "e7f4a0f07be3", \n  "tree": "test-repo"\n}')
 
   $ mozreview exec autoland hg log --encoding=utf-8 /repos/inbound-test-repo/ --template '{rev}:{desc\|firstline}:{phase}\\n'
   2:Bug 1 - \xe3\x81\x93\xe3\x82\x93\xe3\x81\xab\xe3\x81\xa1\xe3\x81\xaf; r=cthulhu:public (esc)
   1:Bug 1 - more goodness; r=cthulhu:public
   0:Bug 1 - some stuff; r=cthulhu:public
 
 Getting status for an unknown job should return a 404
 
--- a/testing/vcttesting/autoland_mach_commands.py
+++ b/testing/vcttesting/autoland_mach_commands.py
@@ -56,29 +56,30 @@ class AutolandCommands(object):
                      help='Autoland user')
     @CommandArgument('--password', required=False, default='autoland',
                      help='Autoland password')
     def post_autoland_job(self, host, tree, rev, destination, pingback_url,
                           trysyntax=None, push_bookmark=None,
                           commit_descriptions=None, ldap_username=None,
                           user=None, password=None):
 
-        if commit_descriptions:
-            commit_descriptions = json.loads(commit_descriptions)
-
         data = {
-            'ldap_username': ldap_username,
             'tree': tree,
             'rev': rev,
             'destination': destination,
-            'trysyntax': trysyntax,
-            'push_bookmark': push_bookmark,
-            'commit_descriptions': commit_descriptions,
             'pingback_url': pingback_url
         }
+        if trysyntax:
+            data['trysyntax'] = trysyntax
+        if push_bookmark:
+            data['push_bookmark'] = push_bookmark
+        if commit_descriptions:
+            data['commit_descriptions'] = json.loads(commit_descriptions)
+        if ldap_username:
+            data['ldap_username'] = ldap_username
 
         host = host.rstrip('/')
         r = requests.post(host + '/autoland', data=json.dumps(data),
                           headers={'Content-Type': 'application/json'},
                           auth=(user, password))
         print(r.status_code, r.text)
 
     @Command('autoland-job-status', category='autoland',