autoland: refactor request validation (bug 1368516) r?smacleod draft
authorbyron jones <glob@mozilla.com>
Tue, 25 Jul 2017 14:48:18 +0800
changeset 11680 fee2d454c1d6ee6d0380ab594d94afca826e7a38
parent 11679 f254a80cbc809b48a48e034c9bdf7703d2868e0b
child 11681 567825c58a84c178b728bff0922968408085f4e0
push id1790
push userbjones@mozilla.com
push dateTue, 19 Sep 2017 04:17:41 +0000
reviewerssmacleod
bugs1368516
autoland: refactor request validation (bug 1368516) r?smacleod Remove repeated error handling code by refactoring validation out into a function. MozReview-Commit-ID: B717LhJ5snK
autoland/autoland/autoland_rest.py
autoland/tests/test-post-autoland-job.t
--- a/autoland/autoland/autoland_rest.py
+++ b/autoland/autoland/autoland_rest.py
@@ -67,16 +67,34 @@ def check_pingback_url(pingback_url):
     # Allow pingbacks to whitelisted hosts from config.json
     for allowed_host in config.get('pingback_allow', []):
         if url.hostname == allowed_host:
             return True
 
     return False
 
 
+def validate_request(request):
+    if request.json is None:
+        raise ValueError('Bad request: missing 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 check_pingback_url(request.json['pingback_url']):
+        raise ValueError('bad pingback_url')
+
+    if not (request.json.get('trysyntax') or
+            request.json.get('commit_descriptions')):
+        raise ValueError('one of trysyntax or commit_descriptions must be '
+                         'specified.')
+
+
 @app.route('/autoland', methods=['POST'])
 def autoland():
     """
     Autoland a patch from one tree to another.
 
     Example request json:
 
     {
@@ -102,54 +120,35 @@ def autoland():
     auth_response = {'WWW-Authenticate': 'Basic realm="Login Required"'}
     if not auth:
         return Response('Login required', 401, auth_response)
     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)
 
-    if request.json is None:
-        error = 'Bad request: missing json'
-        return make_response(jsonify({'error': error}), 400)
-
-    for field in ['ldap_username', 'tree', 'rev', 'destination',
-                  'pingback_url']:
-        if field not in request.json:
-            error = 'Bad request: missing json field: %s' % field
-            return make_response(jsonify({'error': error}), 400)
-
-    if not check_pingback_url(request.json['pingback_url']):
-        error = 'Bad request: bad pingback_url'
-        return make_response(jsonify({'error': error}), 400)
-
-    if not request.json.get('ldap_username'):
-        error = 'Bad request: ldap_username must be specified'
-        return make_response(jsonify({'error': error}), 400)
-
-    if not (request.json.get('trysyntax') or
-            request.json.get('commit_descriptions')):
-        error = ('Bad request: one of trysyntax or commit_descriptions must '
-                 'be specified.')
-        return make_response(jsonify({'error': error}), 400)
+    try:
+        validate_request(request)
+    except ValueError as e:
+        return make_response(jsonify({'error': 'Bad request: %s' % e}), 400)
 
     dbconn = get_dbconn()
     cursor = dbconn.cursor()
 
     query = """
         SELECT created, request->>'ldap_username'
           FROM Transplant
          WHERE landed IS NULL
                AND request->>'rev' = %s
                AND request->>'destination' = %s
     """
     cursor.execute(query, (request.json['rev'], request.json['destination']))
     in_flight = cursor.fetchone()
     if in_flight:
-        error = ('Bad request: a request to land revision %s to %s is already '
+        error = ('Bad Request: a request to land revision %s to %s is already '
                  'in progress'
                  % (request.json['rev'], request.json['destination']))
         app.logger.warn(
             '%s from %s at %s' % (error, in_flight[0], in_flight[1]))
         return make_response(jsonify({'error': error}), 400)
 
     app.logger.info('received transplant request: %s' %
                     json.dumps(request.json))
--- a/autoland/tests/test-post-autoland-job.t
+++ b/autoland/tests/test-post-autoland-job.t
@@ -205,12 +205,12 @@ Post the same job twice.  Start with sto
 guarentee the first request is still in the queue when the second is submitted.
 
   $ PID=`mozreview exec autoland ps x | grep autoland.py | grep -v grep | awk '{ print $1 }'`
   $ mozreview exec autoland kill $PID
   $ REV=`hg log -r . --template "{node|short}"`
   $ ottoland post-autoland-job $AUTOLAND_URL test-repo $REV try http://localhost:9898 --trysyntax "stuff"
   (200, u'{\n  "request_id": 12\n}')
   $ ottoland post-autoland-job $AUTOLAND_URL test-repo $REV try http://localhost:9898 --trysyntax "stuff"
-  (400, u'{\n  "error": "landing revision e7f4a0f07be3 to try is already in progress"\n}')
+  (400, u'{\n  "error": "Bad Request: a request to land revision e7f4a0f07be3 to try is already in progress"\n}')
 
   $ mozreview stop
   stopped 9 containers