Merge lp:~frankban/charms/trusty/juju-gui/custom-port into lp:~juju-gui/charms/trusty/juju-gui/trunk

[Francesco Banconi]

Support user defined port.

Allow specifying a port on which the
GUI server starts listening for requests.

Tests: `make unittest`.

QA:
- bootstrap an environment;
- `make deploy`;
- wait for the unit to be deployed;
- visit the GUI URL, ensure port 80 redirects to 443 as usual
  and the GUI works as usual;
- switch to a customized port: `juju set juju-gui port=8080`;
- ensure the GUI is no longer reachable at port 80 or 443,
  instead it works correctly if you visit `https://<GUI-URL>:8080/`;
- switch to a another customized port: `juju set juju-gui port=443`;
- this time the GUI is still listening on default port for HTTPS,
  but port 80 no longer redirects to HTTPS;
- switch to an invalid port: `juju set juju-gui port=80000`;
- the GUI refuses to do that, and it falls back to using
  the default configuration: ensure port 80 redirects to 443;
- switch again to a customized valid port: `juju set juju-gui port=12345`;
- ensure the GUI server is listening on that port;
- finally, unset the port value: `juju unset juju-gui port`;
- back to defaults: ensure port 80 redirects to 443;
- destroy the environment.

Done, thank you.

https://codereview.appspot.com/174170043/

[Francesco Banconi (frankban)]

Reviewers: mp+241802_code.launchpad.net,

Message:
Please take a look.

Description:
Support user defined port.

Allow specifying a port on which the
GUI server starts listening for requests.

Tests: `make unittest`.

QA:
- bootstrap an environment;
- `make deploy`;
- wait for the unit to be deployed;
- visit the GUI URL, ensure port 80 redirects to 443 as usual
   and the GUI works as usual;
- switch to a customized port: `juju set juju-gui port=8080`;
- ensure the GUI is no longer reachable at port 80 or 443,
   instead it works correctly if you visit `https://<GUI-URL>:8080/`;
- switch to a another customized port: `juju set juju-gui port=443`;
- this time the GUI is still listening on default port for HTTPS,
   but port 80 no longer redirects to HTTPS;
- switch to an invalid port: `juju set juju-gui port=80000`;
- the GUI refuses to do that, and it falls back to using
   the default configuration: ensure port 80 redirects to 443;
- switch again to a customized valid port: `juju set juju-gui
port=12345`;
- ensure the GUI server is listening on that port;
- finally, unset the port value: `juju unset juju-gui port`;
- back to defaults: ensure port 80 redirects to 443;
- destroy the environment.

Done, thank you.

https://code.launchpad.net/~frankban/charms/trusty/juju-gui/custom-port/+merge/241802

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/174170043/

Affected files (+305, -30 lines):
   M README.md
   A [revision details]
   M config.yaml
   M config/guiserver.conf.template
   M hooks/backend.py
   M hooks/utils.py
   M server/guiserver/__init__.py
   M server/guiserver/manage.py
   M server/guiserver/tests/test_manage.py
   M tests/20-functional.test
   M tests/test_backends.py
   M tests/test_utils.py

[Richard Harding (rharding)]

LGTM no QA. Jeff and Francesco thanks! People are going to love getting
this feature into their hands.

https://codereview.appspot.com/174170043/

[Jeff Pihach (hatch)]

LGTM QA OK
This looks great and works great, thanks for taking over!

https://codereview.appspot.com/174170043/

[Francesco Banconi (frankban)]

*** Submitted:

Support user defined port.

Allow specifying a port on which the
GUI server starts listening for requests.

Tests: `make unittest`.

QA:
- bootstrap an environment;
- `make deploy`;
- wait for the unit to be deployed;
- visit the GUI URL, ensure port 80 redirects to 443 as usual
   and the GUI works as usual;
- switch to a customized port: `juju set juju-gui port=8080`;
- ensure the GUI is no longer reachable at port 80 or 443,
   instead it works correctly if you visit `https://<GUI-URL>:8080/`;
- switch to a another customized port: `juju set juju-gui port=443`;
- this time the GUI is still listening on default port for HTTPS,
   but port 80 no longer redirects to HTTPS;
- switch to an invalid port: `juju set juju-gui port=80000`;
- the GUI refuses to do that, and it falls back to using
   the default configuration: ensure port 80 redirects to 443;
- switch again to a customized valid port: `juju set juju-gui
port=12345`;
- ensure the GUI server is listening on that port;
- finally, unset the port value: `juju unset juju-gui port`;
- back to defaults: ensure port 80 redirects to 443;
- destroy the environment.

Done, thank you.

R=rharding, jeff.pihach
CC=
https://codereview.appspot.com/174170043

https://codereview.appspot.com/174170043/

[Francesco Banconi (frankban)]

Thanks for the reviews!

https://codereview.appspot.com/174170043/

=== modified file 'README.md'
--- README.md	2014-04-22 09:27:01 +0000
+++ README.md	2014-11-14 13:53:57 +0000
@@ -191,6 +191,9 @@
 7. It supports deploying local charms by proxying browser HTTPS connections to
    the Juju HTTPS API backend. This also includes retrieving and listing local
    charms' files.
+8. By default, it listens on port 443 for HTTPS secure connections, and
+   redirects port 80 requests to port 443. The port where the server is
+   listening can be changed using the charm configuration "port" option.
 
 ### Legacy server ###
 
@@ -206,7 +209,8 @@
   Use it only if the charm is deployed on a precise machine.
 * The legacy server only provides features 1-4 from the list above. This means
   bundle deployments, timed authentication tokens and local charms are not
-  available when using the legacy configuration.
+  available when using the legacy configuration. Also changing the default port
+  where the server is listening is not supported on the legacy server.
 
 ## Contacting the Developers ##
 

=== modified file 'config.yaml'
--- config.yaml	2014-11-04 01:31:53 +0000
+++ config.yaml	2014-11-14 13:53:57 +0000
@@ -36,7 +36,8 @@
         'https://github.com/mitechie/juju-gui.git @de5e61bf9fa') which will
         clone the git repository and then check out the specified commit
         before building the release.
-      - a URL: The release found at the given URL (ex: http://...) will be deployed.
+      - a URL: The release found at the given URL (ex: http://...) will be
+        deployed.
     type: string
     default: local
   juju-gui-debug:
@@ -49,6 +50,13 @@
       Whether or not the console should be enabled for the browser.
     type: boolean
     default: false
+  port:
+    description: |
+      Supply a different port to host the GUI on besides the default 80 and
+      443. If the provided port is not a valid TCP port (ranging from 1 to
+      65535) the defaults are used.
+    type: int
+    default:
   command-log-file:
     description: |
       The log file where stdout and stderr should be sent for all commands

=== modified file 'config/guiserver.conf.template'
--- config/guiserver.conf.template	2014-08-29 15:17:01 +0000
+++ config/guiserver.conf.template	2014-11-14 13:53:57 +0000
@@ -21,6 +21,9 @@
     --guiroot="{{gui_root}}" \
     --sslpath="{{ssl_cert_path}}" \
     --charmworldurl="{{charmworld_url}}" \
+    {{if port}}
+        --port={{port}} \
+    {{endif}}
     {{if sandbox}}
         --sandbox \
     {{else}}

=== modified file 'hooks/backend.py'
--- hooks/backend.py	2014-02-04 20:07:31 +0000
+++ hooks/backend.py	2014-11-14 13:53:57 +0000
@@ -44,7 +44,6 @@
 
 from charmhelpers import (
     log,
-    open_port,
 )
 
 import utils
@@ -133,9 +132,10 @@
             cached_fonts=config['cached-fonts'], ga_key=config['ga-key'],
             show_get_juju_button=config['show-get-juju-button'],
             password=config.get('password'))
-        # Expose the service.
-        open_port(80)
-        open_port(443)
+        # Set up TCP ports.
+        previous_port = backend.prev_config.get('port')
+        current_port = backend.config.get('port')
+        utils.setup_ports(previous_port, current_port)
 
 
 class ServerInstallMixinBase(object):
@@ -198,7 +198,8 @@
         utils.start_builtin_server(
             build_dir, config['ssl-cert-path'], config['serve-tests'],
             config['sandbox'], config['builtin-server-logging'],
-            not config['secure'], config['charmworld-url'])
+            not config['secure'], config['charmworld-url'],
+            port=config.get('port'))
 
     def stop(self, backend):
         utils.stop_builtin_server()

=== modified file 'hooks/utils.py'
--- hooks/utils.py	2014-09-29 14:52:52 +0000
+++ hooks/utils.py	2014-11-14 13:53:57 +0000
@@ -46,6 +46,7 @@
     'setup_apache_config',
     'setup_gui',
     'setup_haproxy_config',
+    'setup_ports',
     'start_builtin_server',
     'start_haproxy_apache',
     'stop_builtin_server',
@@ -71,8 +72,10 @@
 import tempita
 
 from charmhelpers import (
+    close_port,
     get_config,
     log,
+    open_port,
     RESTART,
     service_control,
     STOP,
@@ -413,6 +416,37 @@
     render_to_file('config.js.template', context, config_js_path)
 
 
+# Simple checker function for port ranges.
+port_in_range = lambda port: 1 <= port <= 65535
+
+
+def setup_ports(previous_port, current_port):
+    """Open or close ports based on the supplied ports.
+
+    The given ports specify the previously provided and the current value.
+    They can be int numbers if the ports are specified, None otherwise, in
+    which case the default ones (80 and 443) are used.
+    """
+    # If a custom port was previously defined we want to make sure we close it.
+    if previous_port is not None and port_in_range(previous_port):
+        log('Closing user provided port {}.'.format(previous_port))
+        close_port(previous_port)
+    if current_port is not None:
+        if port_in_range(current_port):
+            # Ensure the default ports are closed when setting the custom one.
+            log('Closing default ports 80 and 443.')
+            close_port(80)
+            close_port(443)
+            # Open the custom defined port.
+            log('Opening user provided port {}.'.format(current_port))
+            open_port(current_port)
+            return
+        log('Ignoring provided port {}: not in range.'.format(current_port))
+    log('Opening default ports 80 and 443.')
+    open_port(80)
+    open_port(443)
+
+
 def setup_haproxy_config(ssl_cert_path, secure=True):
     """Generate the haproxy configuration file."""
     log('Setting up haproxy Upstart file.')
@@ -513,7 +547,8 @@
 
 def write_builtin_server_startup(
         gui_root, ssl_cert_path, serve_tests=False, sandbox=False,
-        builtin_server_logging='info', insecure=False, charmworld_url=''):
+        builtin_server_logging='info', insecure=False, charmworld_url='',
+        port=None):
     """Generate the builtin server Upstart file."""
     log('Generating the builtin server Upstart file.')
     context = {
@@ -526,7 +561,8 @@
         'charmworld_url': charmworld_url,
         'http_proxy': os.environ.get('http_proxy'),
         'https_proxy': os.environ.get('https_proxy'),
-        'no_proxy': os.environ.get('no_proxy', os.environ.get('NO_PROXY'))
+        'no_proxy': os.environ.get('no_proxy', os.environ.get('NO_PROXY')),
+        'port': port,
     }
     if not sandbox:
         api_url = 'wss://{}'.format(get_api_address())
@@ -542,12 +578,15 @@
 
 def start_builtin_server(
         build_dir, ssl_cert_path, serve_tests, sandbox, builtin_server_logging,
-        insecure, charmworld_url):
+        insecure, charmworld_url, port=None):
     """Start the builtin server."""
+    if (port is not None) and not port_in_range(port):
+        # Do not use the user provided port if it is not valid.
+        port = None
     write_builtin_server_startup(
         build_dir, ssl_cert_path, serve_tests=serve_tests, sandbox=sandbox,
         builtin_server_logging=builtin_server_logging, insecure=insecure,
-        charmworld_url=charmworld_url)
+        charmworld_url=charmworld_url, port=port)
     log('Starting the builtin server.')
     with su('root'):
         service_control(BUILTIN_SERVER, RESTART)

=== modified file 'server/guiserver/__init__.py'
--- server/guiserver/__init__.py	2014-04-09 13:16:08 +0000
+++ server/guiserver/__init__.py	2014-11-14 13:53:57 +0000
@@ -37,7 +37,7 @@
 which originally made the request.
 """
 
-VERSION = (0, 4, 0)
+VERSION = (0, 4, 1)
 
 
 def get_version():

=== modified file 'server/guiserver/manage.py'
--- server/guiserver/manage.py	2014-01-24 15:38:37 +0000
+++ server/guiserver/manage.py	2014-11-14 13:53:57 +0000
@@ -73,6 +73,18 @@
             option_name, ', '.join(choices)))
 
 
+def _validate_range(option_name, min_value, max_value):
+    """Ensure the numeric value passed for the given option is in range.
+
+    The range is defined by min_value and max_value.
+    Exit with an error if the value is not in the given range.
+    """
+    value = options[option_name]
+    if (value is not None) and not (min_value <= value <= max_value):
+        sys.exit('error: the {} argument must be included between '
+                 '{} and {}'.format(option_name, min_value, max_value))
+
+
 def _get_ssl_options():
     """Return a Tornado SSL options dict.
 
@@ -120,11 +132,17 @@
     define(
         'charmworldurl', type=str,
         help='The URL to use for Charmworld.')
+    define(
+        'port', type=int,
+        help='User defined port to run the server on. If no port is defined '
+             'the server will be started on 80 and 443 as per the default '
+             'port options from the charm.')
 
     # In Tornado, parsing the options also sets up the default logger.
     parse_command_line()
     _validate_required('guiroot')
     _validate_choices('apiversion', ('go', 'python'))
+    _validate_range('port', 1, 65535)
     _add_debug(logging.getLogger())
     # Configure the asynchronous HTTP client used by proxy handlers.
     AsyncHTTPClient.configure(
@@ -133,13 +151,19 @@
 
 def run():
     """Run the server"""
+    port = options.port
     if options.insecure:
         # Run the server over an insecure HTTP connection.
-        server().listen(80)
+        if port is None:
+            port = 80
+        server().listen(port)
     else:
         # Default configuration: run the server over a secure HTTPS connection.
-        server().listen(443, ssl_options=_get_ssl_options())
-        redirector().listen(80)
+        if port is None:
+            port = 443
+            redirector().listen(80)
+        server().listen(port, ssl_options=_get_ssl_options())
     version = guiserver.get_version()
     logging.info('starting Juju GUI server v{}'.format(version))
+    logging.info('listening on port {}'.format(port))
     IOLoop.instance().start()

=== modified file 'server/guiserver/tests/test_manage.py'
--- server/guiserver/tests/test_manage.py	2013-08-08 09:20:37 +0000
+++ server/guiserver/tests/test_manage.py	2014-11-14 13:53:57 +0000
@@ -126,6 +126,31 @@
                 manage._validate_choices('arg1', self.choices)
 
 
+class TestValidateRange(ValidatorTestMixin, unittest.TestCase):
+
+    value_range = (1, 10)
+    error = 'error: the {} argument must be included between {} and {}'
+
+    def test_success(self):
+        # The validation passes if the value is included in the range.
+        for value in range(1, 11):
+            with mock.patch('guiserver.manage.options', {'arg1': value}):
+                manage._validate_range('arg1', *self.value_range)
+
+    def test_failure_invalid_range(self):
+        # The validation fails if the value is not in range.
+        error = self.error.format('arg1', *self.value_range)
+        for value in (-50, 0, 11, 100):
+            with mock.patch('guiserver.manage.options', {'arg1': value}):
+                with self.assert_sysexit(error):
+                    manage._validate_range('arg1', *self.value_range)
+
+    def test_success_missing(self):
+        # The validation succeeds if the value is missing.
+        with mock.patch('guiserver.manage.options', {'arg1': None}):
+            manage._validate_range('arg1', *self.value_range)
+
+
 class TestGetSslOptions(unittest.TestCase):
 
     def test_options(self):
@@ -141,6 +166,11 @@
 
 class TestRun(LogTrapTestCase, unittest.TestCase):
 
+    expected_ssl_options = {
+        'certfile': '/my/sslpath/juju.crt',
+        'keyfile': '/my/sslpath/juju.key',
+    }
+
     def mock_and_run(self, **kwargs):
         """Run the application after mocking the IO loop and the options/apps.
 
@@ -149,6 +179,7 @@
         options = {
             'apiversion': 'go',
             'guiroot': '/my/guiroot',
+            'port': None,
             'sslpath': '/my/sslpath',
         }
         options.update(kwargs)
@@ -163,20 +194,33 @@
     def test_secure_mode(self):
         # The application is correctly run in secure mode.
         _, redirector_listen, server_listen = self.mock_and_run(insecure=False)
-        expected_ssl_options = {
-            'certfile': '/my/sslpath/juju.crt',
-            'keyfile': '/my/sslpath/juju.key',
-        }
         redirector_listen.assert_called_once_with(80)
         server_listen.assert_called_once_with(
-            443, ssl_options=expected_ssl_options)
+            443, ssl_options=self.expected_ssl_options)
 
     def test_insecure_mode(self):
         # The application is correctly run in insecure mode.
         _, redirector_listen, server_listen = self.mock_and_run(insecure=True)
-        self.assertEqual(0, redirector_listen.call_count)
+        self.assertFalse(redirector_listen.called)
         server_listen.assert_called_once_with(80)
 
+    def test_customized_port_secure_mode(self):
+        # If the user provided a port, the server starts listening on that port
+        # and the redirector is not used.
+        _, redirector_listen, server_listen = self.mock_and_run(
+            insecure=False, port=8080)
+        self.assertFalse(redirector_listen.called)
+        server_listen.assert_called_once_with(
+            8080, ssl_options=self.expected_ssl_options)
+
+    def test_customized_port_insecure_mode(self):
+        # The application is correctly run in insecure mode with a user
+        # provided port.
+        _, redirector_listen, server_listen = self.mock_and_run(
+            insecure=True, port=12345)
+        self.assertFalse(redirector_listen.called)
+        server_listen.assert_called_once_with(12345)
+
     def test_ioloop_started(self):
         # The IO loop instance is started when the application is run.
         ioloop_start, _, _ = self.mock_and_run()

=== modified file 'tests/20-functional.test'
--- tests/20-functional.test	2014-10-15 07:49:26 +0000
+++ tests/20-functional.test	2014-11-14 13:53:57 +0000
@@ -83,8 +83,6 @@
 
 class DeployTestMixin(object):
 
-    port = '443'
-
     def setUp(self):
         # Perform all graphical operations in memory.
         vdisplay = Xvfb(width=1280, height=720)
@@ -115,14 +113,14 @@
                 error='Browser warning dialog not found.')
             continue_button.click()
 
-    def navigate_to(self, hostname, path='/'):
+    def navigate_to(self, hostname, port=443, path='/'):
         """Load a page using the current Selenium driver.
 
         The page URL is calculated using the provided *hostname* and *path*.
         Retry loading the page until the page is found or a timeout exception
         is raised.
         """
-        base_url = 'https://{}:{}'.format(hostname, self.port)
+        base_url = 'https://{}:{}'.format(hostname, port)
         url = urlparse.urljoin(base_url, path)
 
         def page_ready(driver):
@@ -238,6 +236,15 @@
         self.assertIn('public', cache_directives)
         self.assertIn('must-revalidate', cache_directives)
 
+    def test_different_port(self):
+        # The port on which the server listens can be successfully changed.
+        options = {'port': 8080}
+        self.service_name, unit_info = juju_deploy_gui(options=options)
+        hostname = unit_info['public-address']
+        self.navigate_to(hostname, port=8080)
+        self.handle_browser_warning()
+        self.assertEnvironmentIsConnected()
+
 
 class TestBuiltinServerLocalRelease(DeployTestMixin, unittest.TestCase):
 

=== modified file 'tests/test_backends.py'
--- tests/test_backends.py	2014-02-04 20:07:31 +0000
+++ tests/test_backends.py	2014-11-14 13:53:57 +0000
@@ -169,12 +169,12 @@
             'install_missing_packages': mock.patch(
                 'backend.utils.install_missing_packages'),
             'log': mock.patch('backend.log'),
-            'open_port': mock.patch('backend.open_port'),
             'parse_source': mock.patch(
                 'backend.utils.parse_source', mock_parse_source),
             'save_or_create_certificates': mock.patch(
                 'backend.utils.save_or_create_certificates'),
             'setup_gui': mock.patch('backend.utils.setup_gui'),
+            'setup_ports': mock.patch('backend.utils.setup_ports'),
             'start_builtin_server': mock.patch(
                 'backend.utils.start_builtin_server'),
             'start_haproxy_apache': mock.patch(
@@ -280,7 +280,7 @@
         mocks.compute_build_dir.assert_called_with(
             config['juju-gui-debug'], config['serve-tests'])
         self.assert_write_gui_config_called(mocks, config)
-        mocks.open_port.assert_has_calls([mock.call(80), mock.call(443)])
+        mocks.setup_ports.assert_called_once_with(None, None)
         mocks.start_haproxy_apache.assert_called_once_with(
             mocks.compute_build_dir(), config['serve-tests'],
             self.ssl_cert_path, config['secure'])
@@ -295,14 +295,27 @@
         mocks.compute_build_dir.assert_called_with(
             config['juju-gui-debug'], config['serve-tests'])
         self.assert_write_gui_config_called(mocks, config)
-        mocks.open_port.assert_has_calls([mock.call(80), mock.call(443)])
+        mocks.setup_ports.assert_called_once_with(None, None)
         mocks.start_builtin_server.assert_called_once_with(
             mocks.compute_build_dir(), self.ssl_cert_path,
             config['serve-tests'], config['sandbox'],
             config['builtin-server-logging'], not config['secure'],
-            config['charmworld-url'])
+            config['charmworld-url'], port=None)
         self.assertFalse(mocks.start_haproxy_apache.called)
 
+    def test_start_go_builtin_user_provided_port(self):
+        # Start a juju backend with builtin server and a user provided port.
+        config = self.make_config({'builtin-server': True, 'port': 8080})
+        test_backend = backend.Backend(config=config)
+        with self.mock_all() as mocks:
+            test_backend.start()
+        mocks.setup_ports.assert_called_once_with(None, 8080)
+        mocks.start_builtin_server.assert_called_once_with(
+            mocks.compute_build_dir(), self.ssl_cert_path,
+            config['serve-tests'], config['sandbox'],
+            config['builtin-server-logging'], not config['secure'],
+            config['charmworld-url'], port=8080)
+
     def test_stop_go_legacy(self):
         # Stop a juju-core backend with legacy server.
         config = self.make_config({'builtin-server': False})

=== modified file 'tests/test_utils.py'
--- tests/test_utils.py	2014-02-04 20:07:31 +0000
+++ tests/test_utils.py	2014-11-14 13:53:57 +0000
@@ -48,12 +48,14 @@
     install_missing_packages,
     log_hook,
     parse_source,
+    port_in_range,
     remove_apache_setup,
     remove_haproxy_setup,
     render_to_file,
     save_or_create_certificates,
     setup_apache_config,
     setup_haproxy_config,
+    setup_ports,
     start_builtin_server,
     start_haproxy_apache,
     stop_builtin_server,
@@ -908,6 +910,16 @@
         self.assertNotIn('--sandbox', guiserver_conf)
         self.assertIn('--charmworldurl="http://charmworld.example.com/"',
                       guiserver_conf)
+        # By default the port is not provided to the GUI server.
+        self.assertNotIn('--port', guiserver_conf)
+
+    def test_write_builtin_server_startup_with_port(self):
+        # The builtin server Upstart file is properly generate when a
+        # customized port is provided.
+        write_builtin_server_startup(
+            JUJU_GUI_DIR, self.ssl_cert_path, port=8000)
+        guiserver_conf = self.files['guiserver.conf']
+        self.assertIn('--port=8000', guiserver_conf)
 
     def test_write_builtin_server_startup_sandbox_and_logging(self):
         # The upstart configuration file for the GUI server is correctly
@@ -927,7 +939,7 @@
         start_builtin_server(
             JUJU_GUI_DIR, self.ssl_cert_path, serve_tests=False, sandbox=False,
             builtin_server_logging='info', insecure=False,
-            charmworld_url='http://charmworld.example.com/')
+            charmworld_url='http://charmworld.example.com/', port=443)
         self.assertEqual(self.svc_ctl_call_count, 1)
         self.assertEqual(self.service_names, ['guiserver'])
         self.assertEqual(self.actions, [charmhelpers.RESTART])
@@ -1040,6 +1052,124 @@
         self.assertIn('cachedFonts: true', js_conf)
 
 
+class TestPortInRange(unittest.TestCase):
+
+    def test_valid_port(self):
+        # True is returned if the port is in range.
+        for port in (1, 80, 443, 1234, 8080, 54321, 65535):
+            self.assertTrue(port_in_range(port), port)
+
+    def test_invalid_port(self):
+        # False is returned if the port is not in range.
+        for port in (-10, 0, 65536, 100000):
+            self.assertFalse(port_in_range(port), port)
+
+
[e-mail]('utils.close_port')
[e-mail]('utils.open_port')
[e-mail]('utils.log')
+class TestSetupPorts(unittest.TestCase):
+
+    def test_default_ports(self, mock_log, mock_open_port, mock_close_port):
+        # The default ports are properly opened.
+        setup_ports(None, None)
+        mock_log.assert_called_once_with('Opening default ports 80 and 443.')
+        self.assertEqual(2, mock_open_port.call_count)
+        mock_open_port.assert_has_calls([mock.call(80), mock.call(443)])
+        self.assertFalse(mock_close_port.called)
+
+    def test_from_defaults_to_new_port(
+            self, mock_log, mock_open_port, mock_close_port):
+        # The user provides a customized port.
+        setup_ports(None, 8080)
+        self.assertEqual(2, mock_log.call_count)
+        mock_log.assert_has_calls([
+            mock.call('Closing default ports 80 and 443.'),
+            mock.call('Opening user provided port 8080.'),
+        ])
+        mock_open_port.assert_called_once_with(8080)
+        self.assertEqual(2, mock_close_port.call_count)
+        mock_close_port.assert_has_calls([mock.call(80), mock.call(443)])
+
+    def test_from_previous_port_to_new_port(
+            self, mock_log, mock_open_port, mock_close_port):
+        # The user switches from a previously provided port to a new one.
+        setup_ports(8080, 1234)
+        self.assertEqual(3, mock_log.call_count)
+        mock_log.assert_has_calls([
+            mock.call('Closing user provided port 8080.'),
+            # Always close the default ports in those cases.
+            mock.call('Closing default ports 80 and 443.'),
+            mock.call('Opening user provided port 1234.')
+        ])
+        mock_open_port.assert_called_once_with(1234)
+        self.assertEqual(3, mock_close_port.call_count)
+        mock_close_port.assert_has_calls([
+            mock.call(8080), mock.call(80), mock.call(443)])
+
+    def test_from_previous_port_to_defaults(
+            self, mock_log, mock_open_port, mock_close_port):
+        # The user provided a port and then switches back to defaults.
+        setup_ports(1234, None)
+        self.assertEqual(2, mock_log.call_count)
+        mock_log.assert_has_calls([
+            mock.call('Closing user provided port 1234.'),
+            mock.call('Opening default ports 80 and 443.'),
+        ])
+        self.assertEqual(2, mock_open_port.call_count)
+        mock_open_port.assert_has_calls([mock.call(80), mock.call(443)])
+        mock_close_port.assert_called_once_with(1234)
+
+    def test_from_previous_port_to_invalid(
+            self, mock_log, mock_open_port, mock_close_port):
+        # The user switches from a previously provided port to an invalid one.
+        setup_ports(8080, 0)
+        self.assertEqual(3, mock_log.call_count)
+        mock_log.assert_has_calls([
+            mock.call('Closing user provided port 8080.'),
+            mock.call('Ignoring provided port 0: not in range.'),
+            mock.call('Opening default ports 80 and 443.'),
+        ])
+        self.assertEqual(2, mock_open_port.call_count)
+        mock_open_port.assert_has_calls([mock.call(80), mock.call(443)])
+        mock_close_port.assert_called_once_with(8080)
+
+    def test_from_defaults_to_invalid(
+            self, mock_log, mock_open_port, mock_close_port):
+        # The user provides an invalid port.
+        setup_ports(None, 100000)
+        self.assertEqual(2, mock_log.call_count)
+        mock_log.assert_has_calls([
+            mock.call('Ignoring provided port 100000: not in range.'),
+            mock.call('Opening default ports 80 and 443.'),
+        ])
+        self.assertEqual(2, mock_open_port.call_count)
+        mock_open_port.assert_has_calls([mock.call(80), mock.call(443)])
+        self.assertFalse(mock_close_port.called)
+
+    def test_from_invalid_to_new_port(
+            self, mock_log, mock_open_port, mock_close_port):
+        # The user fixes a previously provided invalid port.
+        setup_ports(123456, 8000)
+        self.assertEqual(2, mock_log.call_count)
+        mock_log.assert_has_calls([
+            mock.call('Closing default ports 80 and 443.'),
+            mock.call('Opening user provided port 8000.')
+        ])
+        mock_open_port.assert_called_once_with(8000)
+        self.assertEqual(2, mock_close_port.call_count)
+        mock_close_port.assert_has_calls([mock.call(80), mock.call(443)])
+
+    def test_from_invalid_to_defaults(
+            self, mock_log, mock_open_port, mock_close_port):
+        # The user switches back to default after providing an invalid port.
+        setup_ports(0, None)
+        mock_log.assert_called_once_with('Opening default ports 80 and 443.')
+        self.assertEqual(2, mock_open_port.call_count)
+        mock_open_port.assert_has_calls([mock.call(80), mock.call(443)])
+        self.assertFalse(mock_close_port.called)
+
+
 @mock.patch('utils.run')
 @mock.patch('utils.log')
 @mock.patch('utils.cmd_log', mock.Mock())