load configuration from the environment

Author: pietroalbini

.gitignore

@@ -1,4 +1,3 @@
 /.env
-config
 *.pyc
 *~

Dockerfile

@@ -14,7 +14,6 @@ COPY setup.py .
 COPY highfive/*.py highfive/
 COPY highfive/configs/ highfive/configs/
 RUN pip install .
-RUN touch highfive/config
 
 EXPOSE 80
 ENV HIGHFIVE_PORT 80

README.md

@@ -162,14 +162,11 @@ your local instance. Here is one approach for running a local server:
   or domain name and port number (if necessary).
 - Obtain an OAuth token. In the account you are creating the token in,
   go to https://github.com/settings/tokens. Grant access to the repository scope.
-- Put the authorization information obtained in the previous step into
-  a file named config in the top of the repository (i.e., the
-  directory containing this file). Here's a template of what it should
-  look like:
+- Put the authorization information obtained in the previous step into a file
+  named `.env` in the top of the repository (i.e., the directory containing
+  this file). Here is a template of what it should look like:
   ```
-  [github]
-  user: OAUTH_TOKEN_USER
-  token: OAUTH_TOKEN
+  HIGHFIVE_GITHUB_TOKEN=your-token
   ```
   _Do not check in this file or commit your OAuth token to a
   repository in any other way. It is a secret._
@@ -189,13 +186,17 @@ Docker
 
 Alternatively, you can build a Docker image that runs Highfive.
 
-    $ docker build -t highfive .
+```
+$ docker build -t highfive .
+```
 
 To run a container, you must mount a config file. Assuming you are
 launching a container from a directory containing a config file, you
 can do the following.
 
-    $ docker run -d --rm --name highfive -p 8080:80 -v $(pwd)/config:/highfive/highfive/config highfive
+```
+$ docker run -d --rm --name highfive -p 8000:80 -e HIGHFIVE_GITHUB_TOKEN=token highfive
+```
 
 At this point, Highfive is accessible at http://localhost:8080.
 

highfive/app.py

@@ -1,5 +1,7 @@
 import json
+import sys
 
+from .config import Config, InvalidTokenException
 from .newpr import HighfiveHandler, UnsupportedRepoError
 from .payload import Payload
 
@@ -9,7 +11,7 @@
 import waitress
 
 
-def create_app():
+def create_app(config):
     app = flask.Flask(__name__)
 
     # The canonical URL is /webhook, but other URLs are accepted for backward
@@ -23,7 +25,7 @@ def new_pr():
         except (KeyError, ValueError), _:
             return 'Error: missing or invalid payload\n', 400
         try:
-            handler = HighfiveHandler(Payload(payload))
+            handler = HighfiveHandler(Payload(payload), config)
             return handler.run(flask.request.headers['X-GitHub-Event'])
         except UnsupportedRepoError:
             return 'Error: this repository is not configured!\n', 400
@@ -37,8 +39,16 @@ def index():
 
 @click.command()
 @click.option('--port', default=8000)
-def cli(port):
-    app = create_app()
+@click.option('--github-token', required=True)
+def cli(port, github_token):
+    try:
+        config = Config(github_token)
+    except InvalidTokenException:
+        print 'error: invalid github token provided!'
+        sys.exit(1)
+    print 'Found a valid GitHub token for user @' + config.github_username
+
+    app = create_app(config)
     waitress.serve(app, port=port)
 
 

highfive/config.py

@@ -0,0 +1,21 @@
+import requests
+
+
+class InvalidTokenException(Exception):
+    pass
+
+
+class Config(object):
+    def __init__(self, github_token):
+        if not github_token:
+            raise InvalidTokenException()
+        self.github_token = github_token
+        self.github_username = self.fetch_github_username()
+
+    def fetch_github_username(self):
+        response = requests.get('https://api.github.com/user', headers={
+            'Authorization': 'token ' + self.github_token
+        })
+        if response.status_code != 200:
+            raise InvalidTokenException()
+        return response.json()['login']

highfive/newpr.py

@@ -47,13 +47,11 @@ class UnsupportedRepoError(IOError):
     pass
 
 class HighfiveHandler(object):
-    def __init__(self, payload):
+    def __init__(self, payload, config):
         self.payload = payload
 
-        self.config = ConfigParser.RawConfigParser()
-        self.config.read('./config')
-        self.integration_user = self.config.get('github', 'user')
-        self.integration_token = self.config.get('github', 'token')
+        self.integration_user = config.github_username
+        self.integration_token = config.github_token
 
         self.repo_config = self.load_repo_config()
 

highfive/tests/test_config.py

@@ -0,0 +1,31 @@
+import pytest
+import responses
+
+from ..config import Config, InvalidTokenException
+
+
+@pytest.mark.unit
+@pytest.mark.hermetic
+class TestConfig(object):
+    def test_empty_token(self):
+        for token in ['', None]:
+            with pytest.raises(InvalidTokenException):
+                Config(token)
+
+    @responses.activate
+    def test_valid_token(self):
+        responses.add(
+            responses.GET, 'https://api.github.com/user',
+            json={'login': 'baz'},
+        )
+
+        config = Config('foobar')
+        assert config.github_token == 'foobar'
+        assert config.github_username == 'baz'
+
+    @responses.activate
+    def test_invalid_token(self):
+        responses.add(responses.GET, 'https://api.github.com/user', status=403)
+
+        with pytest.raises(InvalidTokenException):
+            Config('foobar')

highfive/tests/test_integration_tests.py

@@ -1,9 +1,11 @@
 from highfive import newpr, payload
+from highfive.config import Config
 from highfive.tests import fakes
 from highfive.tests.patcherize import patcherize
 from highfive.tests.test_newpr import HighfiveHandlerMock
 import mock
 import pytest
+import responses
 
 @pytest.mark.integration
 class TestIsNewContributor(object):
@@ -41,6 +43,14 @@ def verify_calls(self):
 
         assert self.mock.call_count == len(self.calls)
 
+def dummy_config():
+    with responses.RequestsMock() as resp:
+        resp.add(
+            responses.GET, 'https://api.github.com/user',
+            json={'login': 'integration-user'},
+        )
+        return Config('integration_token')
+
 @pytest.mark.integration
 @pytest.mark.hermetic
 class TestNewPr(object):
@@ -54,10 +64,6 @@ def make_mocks(cls, patcherize):
 
         cls.mocks['get_irc_nick'].return_value = None
 
-        config_mock = mock.Mock()
-        config_mock.get.side_effect = ('integration-user', 'integration-token')
-        cls.mocks['ConfigParser'].RawConfigParser.return_value = config_mock
-
         cls.mocks['load_json_file'].side_effect = (
             fakes.get_repo_configs()['individuals_no_dirs'],
             fakes.get_global_configs()['base'],
@@ -67,7 +73,7 @@ def test_new_pr_non_contributor(self):
         payload = fakes.Payload.new_pr(
             repo_owner='rust-lang', repo_name='rust', pr_author='pnkfelix'
         )
-        handler = newpr.HighfiveHandler(payload)
+        handler = newpr.HighfiveHandler(payload, dummy_config())
 
         api_req_mock = ApiReqMocker([
             (
@@ -108,7 +114,7 @@ def test_new_pr_empty_body(self):
             repo_owner='rust-lang', repo_name='rust', pr_author='pnkfelix',
             pr_body=None,
         )
-        handler = newpr.HighfiveHandler(payload)
+        handler = newpr.HighfiveHandler(payload, dummy_config())
 
         api_req_mock = ApiReqMocker([
             (
@@ -148,7 +154,7 @@ def test_new_pr_contributor(self):
         payload = fakes.Payload.new_pr(
             repo_owner='rust-lang', repo_name='rust', pr_author='pnkfelix'
         )
-        handler = newpr.HighfiveHandler(payload)
+        handler = newpr.HighfiveHandler(payload, dummy_config())
 
         api_req_mock = ApiReqMocker([
             (
@@ -192,7 +198,7 @@ def test_new_pr_contributor_with_labels(self):
         payload = fakes.Payload.new_pr(
             repo_owner='rust-lang', repo_name='rust', pr_author='pnkfelix'
         )
-        handler = newpr.HighfiveHandler(payload)
+        handler = newpr.HighfiveHandler(payload, dummy_config())
 
         api_req_mock = ApiReqMocker([
             (
@@ -253,7 +259,7 @@ def make_mocks(cls, patcherize):
 
     def test_author_is_commenter(self):
         payload = fakes.Payload.new_comment()
-        handler = newpr.HighfiveHandler(payload)
+        handler = newpr.HighfiveHandler(payload, dummy_config())
         api_req_mock = ApiReqMocker([
             (
                 (
@@ -270,7 +276,7 @@ def test_author_not_commenter_is_collaborator(self):
         payload = fakes.Payload.new_comment()
         payload._payload['issue']['user']['login'] = 'foouser'
 
-        handler = newpr.HighfiveHandler(payload)
+        handler = newpr.HighfiveHandler(payload, dummy_config())
         api_req_mock = ApiReqMocker([
             (
                 (

highfive/tests/test_newpr.py

@@ -1,5 +1,6 @@
 from copy import deepcopy
 from highfive import newpr
+from highfive.config import Config
 from highfive.payload import Payload
 from highfive.tests import fakes
 from highfive.tests.patcherize import patcherize
@@ -8,6 +9,7 @@
 import mock
 import os
 import pytest
+import responses
 from urllib2 import HTTPError
 
 @pytest.mark.unit
@@ -24,27 +26,28 @@ def __init__(
         self.integration_user = integration_user
         self.integration_token = integration_token
 
-        def config_handler(section, key):
-            if section == 'github':
-                if key == 'user':
-                    return integration_user
-                if key == 'token':
-                    return integration_token
-
         self.patchers_stopped = False
+
+        with responses.RequestsMock() as resp:
+            resp.add(
+                responses.GET, 'https://api.github.com/user',
+                json={'login': integration_user},
+            )
+            if integration_token:
+                config = Config(integration_token)
+            else:
+                config = Config('dummy')
+                config.github_token = ''
+
         self.config_patcher = mock.patch('highfive.newpr.ConfigParser')
-        self.mock_config_parser = self.config_patcher.start()
-        self.mock_config = mock.Mock()
-        self.mock_config.get.side_effect = config_handler
-        self.mock_config_parser.RawConfigParser.return_value = self.mock_config
 
         self.load_repo_config_patcher = mock.patch(
             'highfive.newpr.HighfiveHandler.load_repo_config'
         )
         self.mock_load_repo_config = self.load_repo_config_patcher.start()
         self.mock_load_repo_config.return_value = repo_config
 
-        self.handler = newpr.HighfiveHandler(payload)
+        self.handler = newpr.HighfiveHandler(payload, config)
 
     def __enter__(self):
         return self
@@ -58,7 +61,6 @@ def __del__(self):
     def stop_patchers(self):
         if not self.patchers_stopped:
             self.patchers_stopped = True
-            self.config_patcher.stop()
             self.load_repo_config_patcher.stop()
 
 class TestHighfiveHandler(TestNewPR):
@@ -67,11 +69,9 @@ def test_init(self, mock_load_repo_config):
         payload = Payload({'the': 'payload'})
         with HighfiveHandlerMock(payload, repo_config={'a': 'config!'}) as m:
             assert m.handler.payload == payload
-            assert m.handler.config == m.mock_config
             assert m.handler.integration_user == 'integrationUser'
             assert m.handler.integration_token == 'integrationToken'
             assert m.handler.repo_config == {'a': 'config!'}
-            m.mock_config.read.assert_called_once_with('./config')
 
     @mock.patch('highfive.newpr.HighfiveHandler._load_json_file')
     def test_load_repo_config_supported(self, mock_load_json_file):
@@ -1255,22 +1255,19 @@ def test_newpr(self):
         payload = Payload({'action': 'opened'})
         m = self.handler_mock(payload)
         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()
 
     def test_new_comment(self):
         payload = Payload({'action': 'created'})
         m = self.handler_mock(payload)
         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()
 
     def test_unsupported_payload(self):
         payload = Payload({'action': 'something-not-supported'})
         m = self.handler_mock(payload)
         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()

setup.py

@@ -16,6 +16,7 @@
         'click',
         'flask',
         'python-dotenv',
+        'requests',
         'waitress',
     ],
     entry_points = {