Merge lp:~dpb/charms/precise/apache2/vhost-config-relation into lp:charms/apache2

[David Britton]

Add a vhost-config relation to the apache2 charm.

This allows a relating charm to pass the vhosts templates over relation data instead of forcing the juju admin to specify a base64 template in the apache2 juju config. I also stuck in there 'servername' and 'ssl_cert' (if it was self signed), as web apps many times need this data for url writing and for passing on to connecting clients for authentication purposes.

Included with this change are unit tests (make test).

[lp:~dpb/charms/precise/apache2/vhost-config-relation]

75. By David Britton on 2014-05-20

Add test_hooks file (oops)

76. By David Britton on 2014-05-20

extra param removed

77. By David Britton on 2014-05-20

- Adding note about opening non-standard ports to README

78. By David Britton on 2014-05-21

- Add some variables for testing purposes in config-changed
- Add config-changed tests (simple, only testing changed code)
- Fix small error where empty vhosts files were created in all cases

79. By David Britton on 2014-05-21

removing lint

80. By David Britton on 2014-05-21

- fixed failing tests if apache2 was not installed

81. By David Britton on 2014-05-21

- cleanup some language in README, docstrings.

[Charles Butler (lazypower)]

David,

Thank you for this high quality submission! I've tested this with a quick and dirty charm created in python that base64 encodes a jinja2 template and base64 encodes it according to the readme documentation.

It would be great if there were more details with specific details on how to plug this into a charm such as here is the jinja2 vhost template for a basic HTML site being served from /var/www/static - and associated example configuration to do so.

With that being said, the tests pass and it took ~ 20 minutes to write/configure/validate the addition. It performs really well from my perspective.

Thanks again! +1

If you have any questions/comments/concerns about the review contact us in #juju on irc.freenode.net or email the mailing list <email address hidden>

review: Approve

=== modified file 'README.md'
--- README.md	2014-01-30 09:11:53 +0000
+++ README.md	2014-05-21 18:34:21 +0000
@@ -63,6 +63,9 @@
 proxying -- obviously no variables need to be specified if no
 proxying is needed.
 
+Virtual host templates can also be specified via relation.  See the
+vhost-config relation section below for more information.
+
 ### Using the reverseproxy relation
 
 The charm will create the service variable, with the `unit_name`,
@@ -183,6 +186,58 @@
         RewriteRule ^/(.*)$ balancer://gunicorn/$1 [P,L]
     </VirtualHost>
 
+### Using the vhost-config relation
+
+The nice thing about this relation, is as long as a charm support it, deploying
+apache as a front-end for a web service should be as simple as establishing the
+relation.  If you need more details for how to implement this, read on.
+
+The template files themselves can be specified via this relation.  This makes
+deployment of your infrastructure simpler, since users no longer need to
+specify a vhosts config option when using apache2 (though they still can).  A
+candidate charm should provide a relation on the `apache-vhost-config`
+interface.  This charm should simply set the following data when relating:
+
+    relation-set vhosts="- {port: '443', template: dGVtcGxhdGU=}\n- {port: '80', template: dGVtcGxhdGU=}\n"
+
+Notice the `vhosts` definition is in yaml, the format is simple. `vhosts`
+should contain a yaml encoded data structure of a list of key value hashes, or
+dictionaries.  In each dictionary, `port` should be set to the port this vhost
+should listen on, `template` should be set to the base64 encoded template file.
+You can include as many of these dictionaries as you would like.  If you have
+colliding port numbers across your juju infrastructure, the results will be a
+bit unpredictable.
+
+For example, if using python for your relating charm, the code to generate a
+yaml_string for a vhost on port `80` would be similar to this:
+
+    import yaml
+    import base64
+    template = get_template()
+    vhosts = [{"port": "80", "template": base64.b64encode(template)}]
+    yaml_string = yaml.dump(vhosts)
+
+Note, that if you are opening a non-standard port (80 and 443 are opened and
+understood by the default install of apache2 in Ubuntu) you will need to
+instruct Apache to `Listen` on that port in your vhost file.  Something like the
+following will work in your vhost template:
+
+    Listen 8080
+    <VirtualHost *:8080>
+    ...
+    </VirtualHost>
+
+
+#### Relation settings that apache2 provides
+
+When your charm relates it will be provided with the following:
+
+ * `servername` - The Apache2 servername.  This is typically needed by web
+   applications so they know how to write URLs.
+
+ * `ssl_cert` - If you asked for a selfsigned certificate, that cert will
+   be available in this setting as a base64 encoded string.
+
 
 ## Certs, keys and chains
 
 
=== modified file 'hooks/hooks.py'
--- hooks/hooks.py	2014-02-08 15:03:11 +0000
+++ hooks/hooks.py	2014-05-21 18:34:21 +0000
@@ -18,6 +18,8 @@
     log,
     config as orig_config_get,
     relations_of_type,
+    relation_set,
+    relation_ids,
     unit_get
 )
 from charmhelpers.contrib.charmsupport import nrpe
@@ -29,6 +31,7 @@
 service_affecting_packages = ['apache2']
 default_apache22_config_dir = "/etc/apache2/conf.d"
 default_apache24_config_dir = "/etc/apache2/conf-available"
+default_apache_base_dir = "/etc/apache2"
 
 juju_warning_header = """#
 #    "             "
@@ -140,9 +143,9 @@
 
 def site_filename(name, enabled=False):
     if enabled:
-        sites_dir = "/etc/apache2/sites-enabled"
+        sites_dir = "%s/sites-enabled" % default_apache_base_dir
     else:
-        sites_dir = "/etc/apache2/sites-available"
+        sites_dir = "%s/sites-available" % default_apache_base_dir
 
     if is_apache24():
         return "{}/{}.conf".format(sites_dir, name)
@@ -177,6 +180,33 @@
     return True
 
 
+def _get_key_file_location(config_data):
+    """Look at the config, generate the key file location."""
+    key_file = None
+    if config_data['ssl_keylocation']:
+        key_file = '/etc/ssl/private/%s' % \
+            (config_data['ssl_keylocation'].rpartition('/')[2])
+    return key_file
+
+
+def _get_cert_file_location(config_data):
+    """Look at the config, generate the cert file location."""
+    cert_file = None
+    if config_data['ssl_certlocation']:
+        cert_file = '/etc/ssl/certs/%s' % \
+            (config_data['ssl_certlocation'].rpartition('/')[2])
+    return cert_file
+
+
+def _get_chain_file_location(config_data):
+    """Look at the config, generate the chain file location."""
+    chain_file = None
+    if config_data['ssl_chainlocation']:
+        chain_file = '/etc/ssl/certs/%s' % \
+            (config_data['ssl_chainlocation'].rpartition('/')[2])
+    return chain_file
+
+
 ###############################################################################
 # Hook functions
 ###############################################################################
@@ -402,6 +432,41 @@
         logrotate_conf.write(str(template))
 
 
+def create_vhost(port, protocol=None, config_key=None, template_str=None,
+                 config_data={}, relationship_data={}):
+    """
+    Create and enable a vhost in apache.
+
+    @param port: port on which to listen (int)
+    @param protocol: used to name the vhost file intelligently.  If not
+        specified the port will be used instead. (ex: http, https)
+    @param config_key: key in the configuration to look up to
+        retrieve the template.
+    @param template_str: The template itself as a string.
+    @param config_data: juju get-config configuration data.
+    @param relationship_data: if in a relationship, pass in the appropriate
+        structure.  This will be used to inform the template.
+    """
+    if protocol is None:
+        protocol = str(port)
+    close_port(port)
+    if template_str is None:
+        if not config_key or not config_data[config_key]:
+            log("Vhost Template not provided, not configuring: %s" % port)
+            return
+        template_str = config_data[config_key]
+    from jinja2 import Template
+    template = Template(str(base64.b64decode(template_str)))
+    template_data = dict(config_data.items() + relationship_data.items())
+    vhost_name = '%s_%s' % (config_data['servername'], protocol)
+    vhost_file = site_filename(vhost_name)
+    log("Writing file %s with config and relation data" % vhost_file)
+    with open(vhost_file, 'w') as vhost:
+        vhost.write(str(template.render(template_data)))
+    open_port(port)
+    subprocess.call(['/usr/sbin/a2ensite', vhost_name])
+
+
 def config_changed():
     relationship_data = {}
     config_data = config_get()
@@ -435,35 +500,19 @@
         apt_get_purge('apache2-mpm-worker')
     create_mpm_workerfile()
     create_security()
+
     ports = {'http': 80, 'https': 443}
-    for proto in ports.keys():
-        template_var = 'vhost_%s_template' % (proto)
-        template_data = dict(config_data.items() + relationship_data.items())
-        close_port(ports[proto])
-        if template_var in config_data:
-            vhost_name = '%s_%s' % (config_data['servername'], proto)
-            vhost_file = site_filename(vhost_name)
-            from jinja2 import Template
-            template = Template(
-                str(base64.b64decode(config_data[template_var])))
-            log("Writing file %s with config and relation data" % vhost_file)
-            with open(vhost_file, 'w') as vhost:
-                vhost.write(str(template.render(template_data)))
-            open_port(ports[proto])
-        subprocess.call(['/usr/sbin/a2ensite', vhost_name])
+    for protocol, port in ports.iteritems():
+        create_vhost(
+            port,
+            protocol=protocol,
+            config_key="vhost_%s_template" % protocol,
+            config_data=config_data,
+            relationship_data=relationship_data)
 
-    cert_file = None
-    if config_data['ssl_certlocation']:
-        cert_file = '/etc/ssl/certs/%s' % \
-            (config_data['ssl_certlocation'].rpartition('/')[2])
-    key_file = None
-    if config_data['ssl_keylocation']:
-        key_file = '/etc/ssl/private/%s' % \
-            (config_data['ssl_keylocation'].rpartition('/')[2])
-    chain_file = None
-    if config_data['ssl_chainlocation']:
-        chain_file = '/etc/ssl/certs/%s' % \
-            (config_data['ssl_chainlocation'].rpartition('/')[2])
+    cert_file = _get_cert_file_location(config_data)
+    key_file = _get_key_file_location(config_data)
+    chain_file = _get_chain_file_location(config_data)
 
     if cert_file is not None and key_file is not None:
         # ssl_cert is SELFSIGNED so generate self-signed certificate for use.
@@ -567,14 +616,62 @@
         if not os.path.exists('/etc/apache2/security'):
             os.mkdir('/etc/apache2/security', 0755)
         with open('/etc/apache2/security/allowed-ops.txt', 'w') as f:
-            f.write(config_data['openid_provider'].replace(',','\n'))
+            f.write(config_data['openid_provider'].replace(',', '\n'))
             f.write('\n')
             os.chmod(key_file, 0444)
 
+    update_vhost_config_relation()
     update_nrpe_checks()
     ship_logrotate_conf()
 
 
+def update_vhost_config_relation():
+    """
+    Update the vhost file and include the certificate in the relation
+    if it is self-signed.
+    """
+    relation_data = relations_of_type("vhost-config")
+    config_data = config_get()
+    if relation_data is None:
+        return
+
+    for unit_data in relation_data:
+        if "vhosts" in unit_data:
+            all_relation_data = {}
+            all_relation_data.update(
+                get_reverseproxy_data(relation='reverseproxy'))
+            all_relation_data.update(
+                get_reverseproxy_data(relation='website-cache'))
+            try:
+                vhosts = yaml.safe_load(unit_data["vhosts"])
+                for vhost in vhosts:
+                    create_vhost(
+                        vhost["port"],
+                        template_str=vhost["template"],
+                        config_data=config_data,
+                        relationship_data=all_relation_data)
+            except Exception as e:
+                log("Error reading configuration data from relation! %s" % e)
+                raise
+
+    if service_apache2("check"):
+        service_apache2("reload")
+
+    vhost_relation_settings = {
+        "servername": config_data["servername"]}
+
+    cert_file = _get_cert_file_location(config_data)
+    key_file = _get_key_file_location(config_data)
+
+    if cert_file is not None and key_file is not None:
+        if config_data['ssl_cert'] and config_data['ssl_cert'] == "SELFSIGNED":
+            with open(cert_file, 'r') as f:
+                cert = base64.b64encode(f.read())
+            vhost_relation_settings["ssl_cert"] = cert
+    for id in relation_ids("vhost-config"):
+        relation_set(relation_id=id, relation_settings=vhost_relation_settings)
+
+
 def start_hook():
     if service_apache2("status"):
         return(service_apache2("restart"))
@@ -640,6 +737,8 @@
     elif hook_name in ("nrpe-external-master-relation-changed",
                        "local-monitors-relation-changed"):
         update_nrpe_checks()
+    elif hook_name == "vhost-config-relation-changed":
+        config_changed()
     else:
         print "Unknown hook"
         sys.exit(1)
 
=== added file 'hooks/tests/test_config_changed.py'
--- hooks/tests/test_config_changed.py	1970-01-01 00:00:00 +0000
+++ hooks/tests/test_config_changed.py	2014-05-21 18:34:21 +0000
@@ -0,0 +1,73 @@
+from testtools import TestCase
+import hooks
+from mock import patch
+import shutil
+import tempfile
+import os
+
+
+class ConfigChangedTest(TestCase):
+    def setUp(self):
+        super(ConfigChangedTest, self).setUp()
+        self.dirname = tempfile.mkdtemp()
+
+    def tearDown(self):
+        super(ConfigChangedTest, self).tearDown()
+        if os.path.exists(self.dirname):
+            shutil.rmtree(self.dirname)
+
+    @patch('hooks.subprocess.call')
+    @patch('hooks.close_port')
+    @patch('hooks.open_port')
+    @patch('hooks.conf_disable')
+    @patch('hooks.ensure_extra_packages')
+    @patch('hooks.ensure_package_status')
+    @patch('hooks.ship_logrotate_conf')
+    @patch('hooks.update_nrpe_checks')
+    @patch('hooks.run')
+    @patch('hooks.create_security')
+    @patch('hooks.create_mpm_workerfile')
+    @patch('hooks.log')
+    @patch('hooks.relation_ids')
+    @patch('hooks.relations_of_type')
+    @patch('hooks.config_get')
+    @patch('hooks.get_reverseproxy_data')
+    @patch('hooks.service_apache2')
+    @patch('hooks.relation_set')
+    def test_config_changed_ensure_empty_site_dir(
+            self, mock_relation_set, mock_service_apache2,
+            mock_reverseproxy, mock_config_get,
+            mock_relations_of_type, mock_relation_ids, mock_log,
+            mock_create_mpm_workerfile, mock_create_security, mock_run,
+            mock_update_nrpe_checks, mock_ship_logrotate_conf,
+            mock_ensure_package_status, mock_ensure_extra_packages,
+            mock_conf_disable, mock_open_port, mock_close_port, mock_call):
+        """config-changed hook: Site directories should be empty."""
+        mock_config_get.return_value = {
+            "ssl_cert": "",
+            "ssl_key": "",
+            "package_status": "",
+            "enable_modules": "",
+            "disable_modules": "",
+            "mpm_type": "",
+            "ssl_certlocation": "",
+            "ssl_keylocation": "",
+            "ssl_chainlocation": "",
+            "use_rsyslog": "",
+            "config_change_command": "",
+            "openid_provider": "",
+            "servername": "foobar",
+            "vhost_http_template": "",
+            "vhost_https_template": "",
+        }
+        hooks.default_apache_base_dir = self.dirname
+        hooks.default_apache22_config_dir = "%s/conf.d" % self.dirname
+        hooks.default_apache24_config_dir = "%s/conf-available" % self.dirname
+        os.mkdir("%s/sites-enabled" % self.dirname)
+        os.mkdir("%s/sites-available" % self.dirname)
+        os.mkdir("%s/conf.d" % self.dirname)
+        hooks.config_changed()
+        self.assertEqual(
+            len(os.listdir("%s/%s" % (self.dirname, "sites-enabled"))), 0)
+        self.assertEqual(
+            len(os.listdir("%s/%s" % (self.dirname, "sites-available"))), 0)
 
=== added file 'hooks/tests/test_create_vhost.py'
--- hooks/tests/test_create_vhost.py	1970-01-01 00:00:00 +0000
+++ hooks/tests/test_create_vhost.py	2014-05-21 18:34:21 +0000
@@ -0,0 +1,106 @@
+from testtools import TestCase
+import hooks
+import base64
+import tempfile
+from mock import patch
+
+
+class CreateVhostTest(TestCase):
+    def setUp(self):
+        super(CreateVhostTest, self).setUp()
+
+    @patch('hooks.log')
+    @patch('hooks.close_port')
+    def test_create_vhost_no_template(self, mock_close_port, mock_log):
+        """User did not specify a template, error logged."""
+        hooks.create_vhost("80")
+        mock_log.assert_called_once_with(
+            'Vhost Template not provided, not configuring: 80')
+
+    @patch('hooks.close_port')
+    @patch('hooks.site_filename')
+    @patch('hooks.open_port')
+    @patch('hooks.subprocess.call')
+    def test_create_vhost_template_name_port(
+            self, mock_call, mock_open_port, mock_site_filename,
+            mock_close_port):
+        """Check that name generated is sane as a port."""
+        config = {"servername": "unused"}
+        file = tempfile.NamedTemporaryFile()
+        filename = file.name
+        mock_site_filename.return_value = filename
+        hooks.create_vhost(
+            "80",
+            config_data=config,
+            template_str=base64.b64encode("foo"))
+        mock_site_filename.assert_called_once_with("unused_80")
+
+    @patch('hooks.close_port')
+    @patch('hooks.site_filename')
+    @patch('hooks.open_port')
+    @patch('hooks.subprocess.call')
+    def test_create_vhost_template_name_protocol(
+            self, mock_call, mock_open_port, mock_site_filename,
+            mock_close_port):
+        """Check that name generated is sane as a protocol."""
+        config = {"servername": "unused"}
+        file = tempfile.NamedTemporaryFile()
+        filename = file.name
+        mock_site_filename.return_value = filename
+        hooks.create_vhost(
+            "80",
+            protocol="httpfoo",
+            config_data=config,
+            template_str=base64.b64encode("foo"))
+        mock_site_filename.assert_called_once_with("unused_httpfoo")
+
+    @patch('hooks.close_port')
+    @patch('hooks.site_filename')
+    @patch('hooks.open_port')
+    @patch('hooks.subprocess.call')
+    def test_create_vhost_template(
+            self, mock_call, mock_open_port, mock_site_filename,
+            mock_close_port):
+        """
+        Template passed in as string.
+
+        Verify relationship and config inform template as well.
+        """
+        template = ("{{servername}} {{ foo }}")
+        config = {"servername": "test_only"}
+        relationship = {"foo": "bar"}
+        file = tempfile.NamedTemporaryFile()
+        filename = file.name
+        mock_site_filename.return_value = filename
+        hooks.create_vhost(
+            "80",
+            config_data=config,
+            relationship_data=relationship,
+            template_str=base64.b64encode(template))
+        with open(filename, 'r') as f:
+            contents = f.read()
+        self.assertEqual(contents, "test_only bar")
+
+    @patch('hooks.close_port')
+    @patch('hooks.site_filename')
+    @patch('hooks.open_port')
+    @patch('hooks.subprocess.call')
+    def test_create_vhost_template_config(
+            self, mock_call, mock_open_port, mock_site_filename,
+            mock_close_port):
+        """Template passed in as config setting."""
+        template = ("one\n"
+                    "two\n"
+                    "three")
+        config = {"servername": "unused",
+                  "vhost_template": base64.b64encode(template)}
+        file = tempfile.NamedTemporaryFile()
+        filename = file.name
+        mock_site_filename.return_value = filename
+        hooks.create_vhost(
+            "80",
+            config_key="vhost_template",
+            config_data=config)
+        with open(filename, 'r') as f:
+            contents = f.read()
+        self.assertEqual(contents, template)
 
=== added file 'hooks/tests/test_hooks.py'
--- hooks/tests/test_hooks.py	1970-01-01 00:00:00 +0000
+++ hooks/tests/test_hooks.py	2014-05-21 18:34:21 +0000
@@ -0,0 +1,34 @@
+from testtools import TestCase
+import hooks
+
+
+class HooksTest(TestCase):
+    def test__get_key_file_location_empty(self):
+        """No ssl_keylocation, expect None."""
+        self.assertEqual(hooks._get_key_file_location(
+            {"ssl_keylocation": None}), None)
+
+    def test__get_key_file_location(self):
+        """ssl_keylocation, expect correct path."""
+        self.assertEqual(hooks._get_key_file_location(
+            {"ssl_keylocation": "foo"}), "/etc/ssl/private/foo")
+
+    def test__get_cert_file_location_empty(self):
+        """No ssl_keylocation, expect None."""
+        self.assertEqual(hooks._get_cert_file_location(
+            {"ssl_certlocation": None}), None)
+
+    def test__get_cert_file_location(self):
+        """ssl_keylocation, expect correct path."""
+        self.assertEqual(hooks._get_cert_file_location(
+            {"ssl_certlocation": "foo"}), "/etc/ssl/certs/foo")
+
+    def test__get_chain_file_location_empty(self):
+        """No ssl_keylocation, expect None."""
+        self.assertEqual(hooks._get_chain_file_location(
+            {"ssl_chainlocation": None}), None)
+
+    def test__get_chain_file_location(self):
+        """ssl_keylocation, expect correct path."""
+        self.assertEqual(hooks._get_chain_file_location(
+            {"ssl_chainlocation": "foo"}), "/etc/ssl/certs/foo")
 
=== added file 'hooks/tests/test_vhost_config_relation.py'
--- hooks/tests/test_vhost_config_relation.py	1970-01-01 00:00:00 +0000
+++ hooks/tests/test_vhost_config_relation.py	2014-05-21 18:34:21 +0000
@@ -0,0 +1,215 @@
+from testtools import TestCase
+import hooks
+from base64 import b64encode
+from mock import patch, call
+import yaml
+import tempfile
+import os
+import shutil
+
+
+class CreateVhostTest(TestCase):
+
+    def setUp(self):
+        super(CreateVhostTest, self).setUp()
+        self.dirname = tempfile.mkdtemp()
+        os.mkdir("%s/sites-enabled" % self.dirname)
+        os.mkdir("%s/sites-available" % self.dirname)
+        os.mkdir("%s/conf.d" % self.dirname)
+        hooks.default_apache_base_dir = self.dirname
+        hooks.default_apache22_config_dir = "%s/conf.d" % self.dirname
+        hooks.default_apache24_config_dir = "%s/conf-available" % self.dirname
+
+    def tearDown(self):
+        super(CreateVhostTest, self).tearDown()
+        if os.path.exists(self.dirname):
+            shutil.rmtree(self.dirname)
+
+    @patch('hooks.log')
+    @patch('subprocess.call')
+    @patch('hooks.close_port')
+    def test_create_vhost_missing_template(
+            self, mock_close_port, mock_call, mock_log):
+        """Create a vhost file, check contents."""
+        hooks.create_vhost(80)
+        mock_log.assert_called_once_with(
+            "Vhost Template not provided, not configuring: %s" % 80)
+        mock_close_port.assert_called_once()
+        self.assertEqual(
+            len(os.listdir("%s/%s" % (self.dirname, "sites-available"))), 0)
+
+    @patch('hooks.log')
+    @patch('subprocess.call')
+    @patch('hooks.close_port')
+    @patch('hooks.open_port')
+    def test_create_vhost_template_through_config_no_protocol(
+            self, mock_open_port, mock_close_port, mock_call, mock_log):
+        """Create a vhost file, check contents."""
+        template = b64encode("http://{{ variable }}/")
+        config_data = {
+            "template": template,
+            "servername": "test_only"}
+        relationship_data = {
+            "variable": "fantastic"}
+        hooks.create_vhost(
+            80, config_data=config_data, config_key="template",
+            relationship_data=relationship_data)
+        filename = hooks.site_filename("test_only_80")
+        self.assertTrue(os.path.exists(filename))
+        with open(filename, 'r') as file:
+            contents = file.read()
+        self.assertEqual(contents, 'http://fantastic/')
+        mock_open_port.assert_called_once()
+        mock_close_port.assert_called_once()
+        self.assertEqual(
+            len(os.listdir("%s/%s" % (self.dirname, "sites-available"))), 1)
+
+    @patch('hooks.log')
+    @patch('subprocess.call')
+    @patch('hooks.close_port')
+    @patch('hooks.open_port')
+    def test_create_vhost_template_through_config_with_protocol(
+            self, mock_open_port, mock_close_port, mock_call, mock_log):
+        """Create a vhost file, check contents."""
+        template = b64encode("http://{{ variable }}/")
+        config_data = {
+            "template": template,
+            "servername": "test_only"}
+        relationship_data = {
+            "variable": "fantastic"}
+        hooks.create_vhost(
+            80, config_data=config_data, config_key="template",
+            protocol='http', relationship_data=relationship_data)
+        filename = hooks.site_filename("test_only_http")
+        self.assertTrue(os.path.exists(filename))
+        with open(filename, 'r') as file:
+            contents = file.read()
+        self.assertEqual(contents, 'http://fantastic/')
+        mock_open_port.assert_called_once()
+        mock_close_port.assert_called_once()
+        self.assertEqual(
+            len(os.listdir("%s/%s" % (self.dirname, "sites-available"))), 1)
+
+    @patch('hooks.log')
+    @patch('subprocess.call')
+    @patch('hooks.close_port')
+    @patch('hooks.open_port')
+    def test_create_vhost_template_directly(
+            self, mock_open_port, mock_close_port,
+            mock_call, mock_log):
+        """Create a vhost file, check contents."""
+        template = b64encode("http://{{ variable }}/")
+        config_data = {
+            "servername": "test_only"}
+        relationship_data = {
+            "variable": "fantastic"}
+        hooks.create_vhost(
+            80, template_str=template, config_data=config_data,
+            config_key="template", relationship_data=relationship_data)
+        filename = hooks.site_filename("test_only_80")
+        self.assertTrue(os.path.exists(filename))
+        with open(filename, 'r') as file:
+            contents = file.read()
+        self.assertEqual(contents, 'http://fantastic/')
+        mock_open_port.assert_called_once()
+        mock_close_port.assert_called_once()
+        self.assertEqual(
+            len(os.listdir("%s/%s" % (self.dirname, "sites-available"))), 1)
+
+
+class VhostConfigRelationTest(TestCase):
+    @patch('hooks.service_apache2')
+    @patch('hooks.relation_ids')
+    @patch('hooks.relations_of_type')
+    @patch('hooks.get_reverseproxy_data')
+    @patch('hooks.config_get')
+    def test_vhost_config_relation_changed_no_relation_data(
+            self, mock_config_get, mock_relation_get,
+            mock_relations_of_type, mock_relation_ids,
+            mock_service_apache2):
+        """No relation data, do nothing."""
+        mock_relation_get.return_value = None
+        hooks.update_vhost_config_relation()
+
+    @patch('hooks.relations_of_type')
+    @patch('hooks.service_apache2')
+    @patch('hooks.config_get')
+    @patch('hooks.get_reverseproxy_data')
+    @patch('hooks.log')
+    def test_vhost_config_relation_changed_vhost_ports_only(
+            self, mock_log, mock_reverseproxy, mock_config_get,
+            mock_service_apache2, mock_relations_of_type):
+        """vhost_ports only specified, hook should exit with error"""
+        mock_relations_of_type.return_value = [
+            {'vhosts': yaml.dump([{'port': "5555"}])}]
+        mock_config_get.return_value = {}
+        self.assertRaisesRegexp(
+            KeyError, "template", hooks.update_vhost_config_relation)
+
+    @patch('hooks.log')
+    @patch('hooks.relation_ids')
+    @patch('hooks.relations_of_type')
+    @patch('hooks.config_get')
+    @patch('hooks.get_reverseproxy_data')
+    @patch('hooks.create_vhost')
+    @patch('hooks.service_apache2')
+    @patch('hooks.relation_set')
+    def test_vhost_config_relation_changed_vhost_ports_single(
+            self, mock_relation_set, mock_service_apache2,
+            mock_create_vhost, mock_reverseproxy, mock_config_get,
+            mock_relations_of_type, mock_relation_ids, mock_log):
+        """A single vhost entry is created."""
+        mock_relation_ids.return_value = ["testonly"]
+        rel = {"vhosts": yaml.dump([{
+            'port': '80',
+            'template': b64encode("foo")
+        }])}
+        config_data = {
+            "servername": "unused",
+            "ssl_certlocation": "unused",
+            "ssl_keylocation": "unused",
+            "ssl_cert": ""}
+        mock_config_get.return_value = config_data
+        mock_relations_of_type.return_value = [rel]
+        hooks.update_vhost_config_relation()
+        mock_create_vhost.assert_called_once_with(
+            "80",
+            template_str=b64encode("foo"),
+            config_data=config_data,
+            relationship_data={}
+        )
+
+    @patch('hooks.log')
+    @patch('hooks.relation_ids')
+    @patch('hooks.relations_of_type')
+    @patch('hooks.config_get')
+    @patch('hooks.get_reverseproxy_data')
+    @patch('hooks.create_vhost')
+    @patch('hooks.service_apache2')
+    @patch('hooks.relation_set')
+    def test_vhost_config_relation_changed_vhost_ports_multi(
+            self, mock_relation_set, mock_service_apache2,
+            mock_create_vhost, mock_reverseproxy, mock_config_get,
+            mock_relations_of_type, mock_relation_ids, mock_log):
+        """Multiple vhost entries are created."""
+        mock_relation_ids.return_value = ["testonly"]
+        rel = {"vhosts": yaml.dump([
+            {'port': "80", 'template': b64encode("80")},
+            {'port': "443", 'template': b64encode("443")},
+            {'port': "444", 'template': b64encode("444")}])}
+        mock_relations_of_type.return_value = [rel]
+        config_data = {
+            "servername": "unused",
+            "ssl_certlocation": "unused",
+            "ssl_keylocation": "unused",
+            "ssl_cert": ""}
+        mock_config_get.return_value = config_data
+        hooks.update_vhost_config_relation()
+        mock_create_vhost.assert_has_calls([
+            call("80", template_str=b64encode("80"),
+                 config_data=config_data, relationship_data={}),
+            call("443", template_str=b64encode("443"),
+                 config_data=config_data, relationship_data={}),
+            call("444", template_str=b64encode("444"),
+                 config_data=config_data, relationship_data={})])
+        self.assertEqual(mock_create_vhost.call_count, 3)
 
=== added symlink 'hooks/vhost-config-relation-changed'
=== target is u'hooks.py'
=== modified file 'metadata.yaml'
--- metadata.yaml	2013-05-28 20:19:08 +0000
+++ metadata.yaml	2014-05-21 18:34:21 +0000
@@ -27,3 +27,5 @@
     interface: http
   logging:
     interface: syslog
+  vhost-config:
+    interface: apache-vhost-config