Merge pull request #379 from box/enable-https

Enable https for CR master and slave

app/client/build_runner.py

@@ -20,7 +20,7 @@ class BuildRunner(object):
     build results.
 
     Example usage pattern:
-    >>> runner = BuildRunner('http://mymaster.net:123', {'type':'git', 'url':'https://github.com/box/StatusWolf.git'})
+    >>> runner = BuildRunner('https://mymaster.net:123', {'type':'git', 'url':'https://github.com/box/StatusWolf.git'})
     >>> runner.run()
     """
 

app/client/cluster_api_client.py

@@ -3,6 +3,7 @@
 from typing import Any, Callable, Dict, List, Optional
 
 from app.master.build import BuildStatus
+from app.util.conf.configuration import Configuration
 from app.util import log, poll
 from app.util.network import Network
 from app.util.secret import Secret
@@ -15,7 +16,7 @@ class ClusterAPIClient(object):
     """
     def __init__(self, base_api_url):
         """
-        :param base_api_url: The base API url of the service (e.g., 'http://localhost:43000')
+        :param base_api_url: The base API url of the service (e.g., 'http(s)://localhost:43000')
         :type base_api_url: str
         """
         self._api = UrlBuilder(self._ensure_url_has_scheme(base_api_url))
@@ -24,13 +25,13 @@ def __init__(self, base_api_url):
 
     def _ensure_url_has_scheme(self, url):
         """
-        If url does not start with 'http' or 'https', add 'http://' to the beginning.
+        If url does not start with 'http' or 'https', add 'http://' or 'https://' at the beginning.
         :type url: str
         :rtype: str
         """
         url = url.strip()
         if not url.startswith('http'):
-            url = 'http://' + url
+            url = '{}://{}'.format(Configuration['protocol_scheme'], url)
         return url
 
 

app/client/service_runner.py

@@ -119,7 +119,7 @@ def is_up(self, service_url, timeout=0.1):
         timeout_time = time.time() + timeout
         while True:
             try:
-                resp = network.get('http://{}'.format(service_url), timeout=timeout)
+                resp = network.get('{}://{}'.format(Configuration['protocol_scheme'], service_url), timeout=timeout)
                 if resp and resp.ok:
                     return True
             except (requests.RequestException, ConnectionError):

app/subcommands/deploy_subcommand.py

@@ -252,7 +252,7 @@ def _registered_slave_hostnames(self, slave_api_url, network):
 
             # In order to urlparse's 'hostname' attribute to get properly set, the url must start with the scheme
             if not slave_url.startswith('http'):
-                slave_url = 'http://{}'.format(slave_url)
+                slave_url = '{}://{}'.format(Configuration['protocol_scheme'], slave_url)
 
             # Must strip out the port and scheme
             registered_slave_hosts.append(urlparse(slave_url).hostname)

app/subcommands/service_subcommand.py

@@ -2,7 +2,9 @@
 import os
 import sys
 import tornado.ioloop
+from tornado.httpserver import HTTPServer
 
+from app.util.conf.configuration import Configuration
 from app.subcommands.subcommand import Subcommand
 from app.util import fs
 from app.util.safe_thread import SafeThread
@@ -33,7 +35,10 @@ def _start_application(self, application, port):
         # will raise an exception if another process is using the same port. We rely on this exception to force us to
         # exit if there are any port conflicts.
         try:
-            application.listen(port, '0.0.0.0')
+            # If SSL cert and key files are provided in configuration, ClusterRunner wil start with HTTPS protocol.
+            # Otherwise ClusterRunner will start with HTTP protocol.
+            server = HTTPServer(application, ssl_options=self._get_https_options())
+            server.listen(port, '0.0.0.0')
         except OSError:
             self._logger.error('Could not start application on port {}. Is port already in use?'.format(port))
             sys.exit(1)
@@ -54,3 +59,14 @@ def remove_pid_file():
             except OSError:
                 pass
         UnhandledExceptionHandler.singleton().add_teardown_callback(remove_pid_file)
+
+    def _get_https_options(self):
+        https_cert_file = Configuration['https_cert_file']
+        https_key_file = Configuration['https_key_file']
+
+        if https_cert_file and https_key_file:
+            return {
+                'certfile': https_cert_file,
+                'keyfile': https_key_file,
+            }
+        return None

app/util/conf/base_config_loader.py

@@ -5,6 +5,7 @@
 import shutil
 
 from app.util import autoversioning, fs
+from app.util.conf.configuration import Configuration
 from app.util.conf.config_file import ConfigFile
 
 
@@ -98,6 +99,12 @@ def configure_defaults(self, conf):
         # The master must have full clones, as slaves fetch from the master, and one cannot fetch from a shallow clone.
         conf.set('shallow_clones', False)
 
+        # Set the default protocol scheme to 'http'
+        conf.set('protocol_scheme', 'http')
+        # Initialize the SSL cert and key file paths to None
+        conf.set('https_cert_file', None)
+        conf.set('https_key_file', None)
+
     def configure_postload(self, conf):
         """
         After the clusterrunner.conf file has been loaded, generate the paths which descend from the base_directory
@@ -110,6 +117,10 @@ def configure_postload(self, conf):
         conf.set('log_file', join(log_dir, conf.get('log_filename')))
         conf.set('eventlog_file', join(log_dir, conf.get('eventlog_filename')))
 
+        # Set protocol scheme (by default it is set to 'http')
+        if Configuration['https_cert_file'] and Configuration['https_key_file']:
+            conf.set('protocol_scheme', 'https')
+
     def load_from_config_file(self, config, config_filename):
         """
         :type config: Configuration
@@ -144,6 +155,8 @@ def _get_config_file_whitelisted_keys(self):
             'cors_allowed_origins_regex',
             'get_project_from_master',
             'default_http_timeout',
+            'https_cert_file',
+            'https_key_file',
         ]
 
     def _load_section_from_config_file(self, config, config_filename, section):

app/util/network.py

@@ -38,7 +38,8 @@ def reset_session(self):
         if self._session:
             self._session.close()  # Close any pooled connections held by the previous session.
         self._session = requests.Session()
-        self._session.mount('http://', HTTPAdapter(pool_connections=self._poolsize, pool_maxsize=self._poolsize))
+        self._session.mount('{}://'.format(Configuration['protocol_scheme']),
+                            HTTPAdapter(pool_connections=self._poolsize, pool_maxsize=self._poolsize))
 
     def get(self, *args, **kwargs):
         """
@@ -66,6 +67,7 @@ def post_with_digest(self, url, request_params, secret, error_on_failure=False):
         :type url: str
         :type request_params: dict [str, any]
         :param secret: the secret used to produce the message auth digest
+        :param error_on_failure: Boolean to ignore failure
         :rtype: requests.Response
         """
         encoded_body = self.encode_body(request_params)

app/util/url_builder.py

@@ -1,5 +1,6 @@
 import re
 from urllib.parse import urljoin
+from app.util.conf.configuration import Configuration
 
 
 class UrlBuilder(object):
@@ -10,13 +11,13 @@ class UrlBuilder(object):
 
     def __init__(self, service_address, api_version=API_VERSION_1):
         """
-        :param service_address: A host and port and optional scheme, like "http://hostname.example.com:43000"
+        :param service_address: A host and port and optional scheme, like "http(s)://hostname.example.com:43000"
         :type service_address: str
         :type api_version: str
         """
         self._service_address = service_address
         self._api_version = api_version
-        self._scheme = 'http://'
+        self._scheme = '{}://'.format(Configuration['protocol_scheme'])
 
     def url(self, *args):
         """

test/framework/functional/functional_test_cluster.py

@@ -14,6 +14,7 @@
 from app.util import log, poll, process_utils
 from app.util.conf.base_config_loader import BASE_CONFIG_FILE_SECTION
 from app.util.conf.config_file import ConfigFile
+from app.util.conf.configuration import Configuration
 from app.util.secret import Secret
 
 
@@ -391,7 +392,7 @@ def kill(self, kill_gracefully=True):
 
     @property
     def url(self):
-        return 'http://{}:{}'.format(self.host, self.port)
+        return '{}://{}:{}'.format(Configuration['protocol_scheme'], self.host, self.port)
 
     def is_alive(self):
         return self.process.poll() is None

test/unit/test_main.py

@@ -37,6 +37,7 @@ def setUp(self):
         self.patch('app.util.conf.base_config_loader.platform').node.return_value = self._HOSTNAME
         self.patch('app.subcommands.master_subcommand.analytics.initialize')
         self.patch('argparse._sys.stderr')  # Hack to prevent argparse from printing output during tests.
+        self.patch('app.subcommands.service_subcommand.HTTPServer')
 
         # We want the method _start_app_force_kill_countdown mocked out for every test *except* one, so we are patching
         # this method in an uglier way that allows us to unpatch it just for that test.

test/unit/util/conf/test_master_config_loader.py

@@ -0,0 +1,48 @@
+from app.util.conf.master_config_loader import MasterConfigLoader
+from app.util.conf.configuration import Configuration
+from test.framework.base_unit_test_case import BaseUnitTestCase
+
+
+class TestMasterConfigLoader(BaseUnitTestCase):
+
+    def test_configure_default_sets_protocol_scheme_to_http(self):
+        mock_config_file = self.patch('app.util.conf.base_config_loader.ConfigFile').return_value
+
+        config = Configuration.singleton()
+        config_loader = MasterConfigLoader()
+        config_loader.configure_defaults(config)
+
+        key = 'protocol_scheme'
+        expected_stored_protocol_scheme_value = 'http'
+        actual_stored_protocol_scheme_value = Configuration[key]
+
+        self.assertEqual(expected_stored_protocol_scheme_value, actual_stored_protocol_scheme_value,
+                         'The configuration value for the key "{}" was expected to be {}:{}, but was {}:{}.'.format(
+                             key, type(expected_stored_protocol_scheme_value), expected_stored_protocol_scheme_value,
+                             type(actual_stored_protocol_scheme_value), actual_stored_protocol_scheme_value))
+
+    def test_configure_postload_sets_protocol_scheme_to_https(self):
+        mock_config_file = self.patch('app.util.conf.base_config_loader.ConfigFile').return_value
+        mock_config_file.read_config_from_disk.return_value = {'general': {'https_cert_file': '/path/to/cert',
+                                                                           'https_key_file': '/path/to/key'},
+                                                               'master': {}
+                                                              }
+        # Unpached the patched method in BaseUnitTestCase for only this test case
+        # load_from_config_file is needed to update the Configuration with above "https_cert_file" and
+        # "https_key_file" values
+        self.unpatch('app.util.conf.master_config_loader.MasterConfigLoader.load_from_config_file')
+
+        config = Configuration.singleton()
+        config_loader = MasterConfigLoader()
+        config_loader.configure_defaults(config)
+        config_loader.load_from_config_file(config, config_filename='fake_filename')
+        config_loader.configure_postload(config)
+
+        key = 'protocol_scheme'
+        expected_stored_protocol_scheme_value = 'https'
+        actual_stored_protocol_scheme_value = Configuration[key]
+
+        self.assertEqual(expected_stored_protocol_scheme_value, actual_stored_protocol_scheme_value,
+                         'The configuration value for the key "{}" was expected to be {}:{}, but was {}:{}.'.format(
+                             key, type(expected_stored_protocol_scheme_value), expected_stored_protocol_scheme_value,
+                             type(actual_stored_protocol_scheme_value), actual_stored_protocol_scheme_value))

test/unit/util/conf/test_slave_config_loader.py

@@ -0,0 +1,44 @@
+from app.util.conf.slave_config_loader import SlaveConfigLoader
+from app.util.conf.configuration import Configuration
+from test.framework.base_unit_test_case import BaseUnitTestCase
+
+
+class TestSlaveConfigLoader(BaseUnitTestCase):
+
+    def test_configure_default_sets_protocol_scheme_to_http(self):
+        mock_config_file = self.patch('app.util.conf.base_config_loader.ConfigFile').return_value
+
+        config = Configuration.singleton()
+        config_loader = SlaveConfigLoader()
+        config_loader.configure_defaults(config)
+
+        key = 'protocol_scheme'
+        expected_stored_protocol_scheme_value = 'http'
+        actual_stored_protocol_scheme_value = Configuration[key]
+
+        self.assertEqual(expected_stored_protocol_scheme_value, actual_stored_protocol_scheme_value,
+                         'The configuration value for the key "{}" was expected to be {}:{}, but was {}:{}.'.format(
+                             key, type(expected_stored_protocol_scheme_value), expected_stored_protocol_scheme_value,
+                             type(actual_stored_protocol_scheme_value), actual_stored_protocol_scheme_value))
+
+    def test_configure_postload_sets_protocol_scheme_to_https(self):
+        mock_config_file = self.patch('app.util.conf.base_config_loader.ConfigFile').return_value
+        mock_config_file.read_config_from_disk.return_value = {'general': {'https_cert_file': '/path/to/cert',
+                                                                           'https_key_file': '/path/to/key'},
+                                                               'slave': {}
+                                                              }
+
+        config = Configuration.singleton()
+        config_loader = SlaveConfigLoader()
+        config_loader.configure_defaults(config)
+        config_loader.load_from_config_file(config, config_filename='fake_filename')
+        config_loader.configure_postload(config)
+
+        key = 'protocol_scheme'
+        expected_stored_protocol_scheme_value = 'https'
+        actual_stored_protocol_scheme_value = Configuration[key]
+
+        self.assertEqual(expected_stored_protocol_scheme_value, actual_stored_protocol_scheme_value,
+                         'The configuration value for the key "{}" was expected to be {}:{}, but was {}:{}.'.format(
+                             key, type(expected_stored_protocol_scheme_value), expected_stored_protocol_scheme_value,
+                             type(actual_stored_protocol_scheme_value), actual_stored_protocol_scheme_value))