move to a flask webserver instead of cgi

Author: pietroalbini

Dockerfile

@@ -1,29 +1,22 @@
 FROM ubuntu:bionic
 
-RUN apt-get update && apt-get install -y apache2 \
-  ca-certificates \
-  python
+RUN apt-get update && DEBIAN_FRONTEND=noninteractive apt-get install --no-install-recommends -y \
+    ca-certificates \
+    python \
+    python-setuptools \
+    python-wheel \
+    python-pip
 
-# Set up Highfive
 RUN mkdir /highfive
 WORKDIR /highfive
 
 COPY setup.py .
 COPY highfive/*.py highfive/
 COPY highfive/configs/ highfive/configs/
-RUN python setup.py install
+RUN pip install .
 RUN touch highfive/config
-RUN chown -R www-data:www-data .
-
-# Set up Apache
-WORKDIR /etc/apache2
-COPY deployment/highfive.conf conf-available/highfive.conf
-RUN a2enmod cgi
-RUN rm conf-enabled/serve-cgi-bin.conf
-RUN rm sites-enabled/*
-RUN ln -s ../conf-available/highfive.conf conf-enabled
-
-RUN mkdir /var/log/highfive-tracebacks && chown www-data: /var/log/highfive-tracebacks
 
 EXPOSE 80
-CMD apachectl -D FOREGROUND
+ENV HIGHFIVE_PORT 80
+
+CMD highfive

README.md

@@ -142,13 +142,21 @@ Local Development
 You can run Highfive on your machine and configure a repository to use
 your local instance. Here is one approach for running a local server:
 
-- Use [serve.py](/serve.py) to run the Highfive service. From the
-  repository root, do:
+- Create a [virtualenv](https://virtualenv.pypa.io/en/stable/) to isolate the
+  Python environment from the rest of the system, and install highfive in it:
   ```
-  $ PYTHONPATH=$PYTHONPATH:$PWD python serve.py
+  $ virtualenv -p python2 env
+  $ env/bin/pip install -e .
   ```
-  Now you have Highfive listening on port 8000 of your machine.
-- Your Highfive instance will need to be reachable from outside of your machine. One way to do this is to use [ngrok](https://ngrok.com/) to get a temporary domain name that proxies to your Highfive instance. Additionally, you will be able to use ngrok's inspector to easily examine and replay the requests.
+- Run the highfive command to start a development server on port 8000:
+  ```
+  $ env/bin/highfive
+  ```
+- Your Highfive instance will need to be reachable from outside of your
+  machine. One way to do this is to use [ngrok](https://ngrok.com/) to get a
+  temporary domain name that proxies to your Highfive instance. Additionally,
+  you will be able to use ngrok's inspector to easily examine and replay the
+  requests.
 - Set up the webhook by following the instructions in [Enabling a
   Repo](#enabling-a-repo), substituting your local Highfive IP address
   or domain name and port number (if necessary).

deployment/highfive.conf

@@ -0,0 +1,45 @@
+import json
+
+from .newpr import HighfiveHandler, UnsupportedRepoError
+from .payload import Payload
+
+import click
+import flask
+import waitress
+
+
+def create_app():
+    app = flask.Flask(__name__)
+
+    # The canonical URL is /webhook, but other URLs are accepted for backward
+    # compatibility.
+    @app.route("/webhook", methods=['POST'])
+    @app.route("/newpr.py", methods=['POST'])
+    @app.route("/highfive/newpr.py", methods=['POST'])
+    def new_pr():
+        try:
+            payload = json.loads(flask.request.form['payload'])
+        except (KeyError, ValueError), _:
+            return 'Error: missing or invalid payload\n', 400
+        try:
+            handler = HighfiveHandler(Payload(payload))
+            return handler.run(flask.request.headers['X-GitHub-Event'])
+        except UnsupportedRepoError:
+            return 'Error: this repository is not configured!\n', 400
+
+    @app.route('/')
+    def index():
+        return 'Welcome to highfive!\n'
+
+    return app
+
+
+@click.command()
+@click.option('--port', default=8000)
+def cli(port):
+    app = create_app()
+    waitress.serve(app, port=port)
+
+
+def main():
+    cli(auto_envvar_prefix='HIGHFIVE')

highfive/app.py

@@ -6,7 +6,6 @@
 from copy import deepcopy
 import json
 import random
-import sys
 import ConfigParser
 from StringIO import StringIO
 import gzip
@@ -66,14 +65,17 @@ def load_repo_config(self):
         except IOError:
             raise UnsupportedRepoError
 
-    def run(self):
-        if self.payload["action"] == "opened":
+    def run(self, event):
+        if event == "ping":
+            return "Ping received! The webhook is configured correctly!\n"
+        elif event == "pull_request" and self.payload["action"] == "opened":
             self.new_pr()
-        elif self.payload["action"] == "created":
+            return 'OK\n'
+        elif event == "issue_comment" and self.payload["action"] == "created":
             self.new_comment()
+            return 'OK\n'
         else:
-            print self.payload["action"]
-            sys.exit(0)
+            return 'Unsupported webhook event.\n'
 
     def _load_json_file(self, name):
         configs_dir = os.path.join(os.path.dirname(__file__), 'configs')
@@ -419,18 +421,3 @@ def new_comment(self):
                 reviewer, owner, repo, issue, self.integration_user,
                 author, None
             )
-
-
-if __name__ == "__main__":
-    print "Content-Type: text/html;charset=utf-8"
-    print
-
-    cgitb.enable(display=0, logdir="/var/log/highfive-tracebacks", format="txt")
-
-    post = cgi.FieldStorage()
-    payload_raw = post.getfirst("payload",'')
-    try:
-        handler = HighfiveHandler(payload.Payload(json.loads(payload_raw)))
-        handler.run()
-    except UnsupportedRepoError:
-        print "Unsupported repository"

highfive/newpr.py

@@ -1244,7 +1244,6 @@ def make_mocks(cls, patcherize):
         cls.mocks = patcherize((
             ('new_pr', 'highfive.newpr.HighfiveHandler.new_pr'),
             ('new_comment', 'highfive.newpr.HighfiveHandler.new_comment'),
-            ('sys', 'highfive.newpr.sys'),
         ))
 
     def handler_mock(self, payload):
@@ -1255,26 +1254,23 @@ def handler_mock(self, payload):
     def test_newpr(self):
         payload = Payload({'action': 'opened'})
         m = self.handler_mock(payload)
-        m.handler.run()
+        assert m.handler.run('pull_request') == 'OK\n'
         assert m.mock_config.get.call_count == 2
         self.mocks['new_pr'].assert_called_once_with()
         self.mocks['new_comment'].assert_not_called()
-        self.mocks['sys'].exit.assert_not_called()
 
     def test_new_comment(self):
         payload = Payload({'action': 'created'})
         m = self.handler_mock(payload)
-        m.handler.run()
+        assert m.handler.run('issue_comment') == 'OK\n'
         assert m.mock_config.get.call_count == 2
         self.mocks['new_pr'].assert_not_called()
         self.mocks['new_comment'].assert_called_once_with()
-        self.mocks['sys'].exit.assert_not_called()
 
     def test_unsupported_payload(self):
         payload = Payload({'action': 'something-not-supported'})
         m = self.handler_mock(payload)
-        m.handler.run()
+        assert m.handler.run('issue_comment') != 'OK\n'
         assert m.mock_config.get.call_count == 2
         self.mocks['new_pr'].assert_not_called()
         self.mocks['new_comment'].assert_not_called()
-        self.mocks['sys'].exit.assert_called_once_with(0)

highfive/tests/test_newpr.py

@@ -12,4 +12,15 @@
     packages = [
         'highfive',
     ],
+    install_requires = [
+        'click',
+        'flask',
+        'waitress',
+    ],
+    entry_points = {
+        'console_scripts': [
+            'highfive=highfive.app:main',
+        ],
+    },
+    zip_safe = False,
 )