Merge lp:~tribaal/landscape-charm/add-root-url into lp:~landscape/landscape-charm/trunk

[Chris Glass]

This branch introduces a root-url global configuration item, and sets it to either the
value of the root-url charm configuration option or (if it's not present), the
haproxy's public-address (as set throught the website relation).

This branch introduces a root-url global configuration item, and sets it to either the
value of the root-url charm configuration option or (if it's not present), the
haproxy's public-address (as set throught the website relation).

[🤖 Landscape Builder (landscape-builder)]

Command: make ci-test
Result: Success
Revno: 252
Branch: lp:~tribaal/landscape-charm/add-root-url
Jenkins: https://ci.lscape.net/job/latch-test/520/

review: Approve (test results)

[🤖 Landscape Builder (landscape-builder)]

Command: make ci-test
Result: Success
Revno: 253
Branch: lp:~tribaal/landscape-charm/add-root-url
Jenkins: https://ci.lscape.net/job/latch-test/522/

review: Approve (test results)

[Free Ekanayaka (free.ekanayaka)]

Looks good, however I have blocking comment, see below.

review: Needs Fixing

[Benji York (benji)]

This branch looks good.

review: Approve

[lp:~tribaal/landscape-charm/add-root-url]

254. By Chris Glass on 2015-04-23

Addressed review comments, refactoring.

[Chris Glass (tribaal)]

All comments addressed.

[Free Ekanayaka (free.ekanayaka)]

Thanks! +1 with a couple of minors

review: Approve

[lp:~tribaal/landscape-charm/add-root-url]

255. By Chris Glass on 2015-04-23

Fixed last comments.

[Chris Glass (tribaal)]

Fixed last comments.

[lp:~tribaal/landscape-charm/add-root-url]

256. By Chris Glass on 2015-04-23

lint

=== modified file 'config.yaml'
--- config.yaml	2015-04-13 10:57:02 +0000
+++ config.yaml	2015-04-23 14:34:15 +0000
@@ -48,6 +48,12 @@
             Has no effect if changed after a deployment.
         type: string
         default:
+    root-url:
+        description: |
+            The root URL for this landscape deployment. If left blank the public
+            IP of the first related HAproxy unit will be used instead.
+        type: string
+        default: ""
     ssl-cert:
         type: string
         description: |
 
=== added file 'hooks/lib/relations/config.py'
--- hooks/lib/relations/config.py	1970-01-01 00:00:00 +0000
+++ hooks/lib/relations/config.py	2015-04-23 14:34:15 +0000
@@ -0,0 +1,23 @@
+from charmhelpers.core import hookenv
+from lib.hook import HookError
+from lib.utils import is_valid_url
+
+
+class ConfigRequirer(dict):
+    """Dependency manager for the service configuration.
+
+    Take care of validating and exposing configuration values for use
+    in service manager callbacks."""
+
+    def __init__(self, hookenv=hookenv):
+        config = hookenv.config()
+        self._validate(config)
+        self.update({"config": config})
+
+    def _validate(self, config):
+        root_url = config.get("root-url")
+        if root_url and not is_valid_url(root_url):
+            raise HookError(
+                "The 'root-url' configuration value is not a valid URL."
+                " Please make sure it is of the form"
+                " 'http[s]://<hostname>/'")
 
=== modified file 'hooks/lib/relations/haproxy.py'
--- hooks/lib/relations/haproxy.py	2015-04-17 09:48:05 +0000
+++ hooks/lib/relations/haproxy.py	2015-04-23 14:34:15 +0000
@@ -94,7 +94,8 @@
             "crts": self._get_ssl_certificate(),
             "servers": [self._get_server("appserver")],
             "backends": [
-                self._get_backend("message", [self._get_server("message-server")]),
+                self._get_backend(
+                    "message", [self._get_server("message-server")]),
                 self._get_backend("api", [self._get_server("api")]),
             ],
         })
 
=== added file 'hooks/lib/relations/tests/test_config.py'
--- hooks/lib/relations/tests/test_config.py	1970-01-01 00:00:00 +0000
+++ hooks/lib/relations/tests/test_config.py	2015-04-23 14:34:15 +0000
@@ -0,0 +1,59 @@
+from lib.tests.helpers import HookenvTest
+from lib.relations.config import ConfigRequirer
+from lib.hook import HookError
+
+
+class ServicesHookTest(HookenvTest):
+
+    def test_root_url_is_set_and_invalid(self):
+        """
+        If an invalid root-url config option is set, a HookeError is raised.
+        """
+        self.hookenv.config().update({"root-url": "asd"})
+
+        with self.assertRaises(HookError) as error:
+            ConfigRequirer(hookenv=self.hookenv)
+
+        expected = ("The 'root-url' configuration value is not a valid URL."
+                    " Please make sure it is of the form"
+                    " 'http[s]://<hostname>/'")
+        self.assertEqual(expected, error.exception.message)
+
+    def test_root_url_is_set_without_protocol(self):
+        """
+        If an invalid root-url config option is set without a protocol, a
+        HookError is raised.
+        """
+        self.hookenv.config().update({"root-url": "example.com/"})
+        with self.assertRaises(HookError) as error:
+            ConfigRequirer(hookenv=self.hookenv)
+
+        expected = ("The 'root-url' configuration value is not a valid URL."
+                    " Please make sure it is of the form"
+                    " 'http[s]://<hostname>/'")
+        self.assertEqual(expected, error.exception.message)
+
+    def test_root_url_is_set_without_trailing_slash(self):
+        """
+        If an invalid root-url config option is set without a trailing slash,
+        a HookError is raised.
+        """
+        self.hookenv.config().update({"root-url": "https://example.com"})
+        with self.assertRaises(HookError) as error:
+            ConfigRequirer(hookenv=self.hookenv)
+
+        expected = ("The 'root-url' configuration value is not a valid URL."
+                    " Please make sure it is of the form"
+                    " 'http[s]://<hostname>/'")
+        self.assertEqual(expected, error.exception.message)
+
+    def test_a_valid_root_url_configuration_key_is_set(self):
+        """No excpetion is raised if the root-url config entry is valid.
+
+        The resulting ConfigRequirer is a dict with a "config" entry that
+        matches the contents of the config passed.
+        """
+        self.hookenv.config().update({"root-url": "https://example.com/"})
+        result = ConfigRequirer(hookenv=self.hookenv)
+        self.assertEqual({"config": {"root-url": "https://example.com/"}},
+                         result)
 
=== modified file 'hooks/lib/relations/tests/test_landscape.py'
--- hooks/lib/relations/tests/test_landscape.py	2015-04-17 09:40:16 +0000
+++ hooks/lib/relations/tests/test_landscape.py	2015-04-23 14:34:15 +0000
@@ -138,7 +138,7 @@
                            "secret-token": "old-token",
                            "leader-ip": "old-ip"}))
         context = LandscapeLeaderContext(
-                host=self.host, path=self.path, hookenv=self.hookenv)
+            host=self.host, path=self.path, hookenv=self.hookenv)
         self.assertEqual({"database-password": "old-sekret",
                           "secret-token": "old-token",
                           "leader-ip": "old-ip"}, context)
 
=== modified file 'hooks/lib/services.py'
--- hooks/lib/services.py	2015-04-17 11:34:11 +0000
+++ hooks/lib/services.py	2015-04-23 14:34:15 +0000
@@ -13,6 +13,7 @@
     HAProxyProvider, HAProxyRequirer, OFFLINE_FOLDER)
 from lib.relations.landscape import (
     LandscapeLeaderContext, LandscapeRequirer, LandscapeProvider)
+from lib.relations.config import ConfigRequirer
 from lib.relations.hosted import HostedRequirer
 from lib.callbacks.scripts import SchemaBootstrap, LSCtl
 from lib.callbacks.filesystem import (
@@ -60,12 +61,12 @@
             # Required data is available to the render_template calls below.
             "required_data": [
                 LandscapeRequirer(leader_context),
+                ConfigRequirer(self._hookenv),
                 PostgreSQLRequirer(),
                 RabbitMQRequirer(),
                 HAProxyRequirer(),
                 HostedRequirer(),
-                {"config": hookenv.config(),
-                 "is_leader": is_leader},
+                {"is_leader": is_leader},
             ],
             "data_ready": [
                 render_template(
 
=== modified file 'hooks/lib/tests/test_templates.py'
--- hooks/lib/tests/test_templates.py	2015-04-20 08:42:13 +0000
+++ hooks/lib/tests/test_templates.py	2015-04-23 14:34:15 +0000
@@ -4,7 +4,7 @@
 from lib.tests.helpers import TemplateTest
 from lib.tests.sample import (
     SAMPLE_DB_UNIT_DATA, SAMPLE_LEADER_CONTEXT_DATA, SAMPLE_AMQP_UNIT_DATA,
-    SAMPLE_HOSTED_DATA)
+    SAMPLE_HOSTED_DATA, SAMPLE_WEBSITE_UNIT_DATA)
 
 
 class ServiceConfTest(TemplateTest):
@@ -16,6 +16,7 @@
         self.context = {
             "db": [SAMPLE_DB_UNIT_DATA.copy()],
             "amqp": [SAMPLE_AMQP_UNIT_DATA.copy()],
+            "haproxy": SAMPLE_WEBSITE_UNIT_DATA,
             "leader": SAMPLE_LEADER_CONTEXT_DATA.copy(),
             "hosted": [SAMPLE_HOSTED_DATA.copy()],
             "config": {},
@@ -93,6 +94,29 @@
         config.readfp(buffer)
         self.assertEqual("1.2.3.4", config.get("package-search", "host"))
 
+    def test_render_with_haproxy_address_as_root_url(self):
+        """
+        The service.conf file has root-url set to the haproxy public IP if the
+        config doesn't have a root-url entry.
+        """
+        self.context["haproxy"]["public-address"] = "4.3.2.1"
+
+        buffer = StringIO(self.template.render(self.context))
+        config = ConfigParser()
+        config.readfp(buffer)
+        self.assertEqual("https://4.3.2.1/", config.get("global", "root-url"))
+
+    def test_render_with_config_provided_root_url(self):
+        """
+        The service.conf file has root-url set to the content of the root-url
+        charm config option if it is specified.
+        """
+        self.context["config"]["root-url"] = "https://8.8.8.8/"
+        buffer = StringIO(self.template.render(self.context))
+        config = ConfigParser()
+        config.readfp(buffer)
+        self.assertEqual("https://8.8.8.8/", config.get("global", "root-url"))
+
 
 class LandscapeDefaultsTest(TemplateTest):
 
 
=== added file 'hooks/lib/tests/test_utils.py'
--- hooks/lib/tests/test_utils.py	1970-01-01 00:00:00 +0000
+++ hooks/lib/tests/test_utils.py	2015-04-23 14:34:15 +0000
@@ -0,0 +1,29 @@
+import unittest
+
+from lib.utils import is_valid_url
+
+
+class IsValidURLTestCase(unittest.TestCase):
+
+    def test_missing_trailing_slash(self):
+        """No trailing slash means the URL is invalid for root-url."""
+        self.assertFalse(is_valid_url("https://google.com"))
+
+    def test_missing_protocol(self):
+        """A protocol (http or https) is needed for a URL to be a valid
+        root-url."""
+        self.assertFalse(is_valid_url("ftp://google.com/"))
+
+    def test_http_or_https_protocol(self):
+        """The URL for the root-url should have either http or https as a
+        protocol."""
+        self.assertTrue(is_valid_url("http://google.com/"))
+        self.assertTrue(is_valid_url("https://google.com/"))
+
+    def test_missing_protocol_delimiter(self):
+        """We need the URL to have the protocol field"""
+        self.assertFalse(is_valid_url("httpisawesome.com/"))
+
+    def test_correct_url_is_validated(self):
+        """A "correct" URL is validated by the is_valid_url function."""
+        self.assertTrue(is_valid_url("http://example.com:9090/blah/"))
 
=== added file 'hooks/lib/utils.py'
--- hooks/lib/utils.py	1970-01-01 00:00:00 +0000
+++ hooks/lib/utils.py	2015-04-23 14:34:15 +0000
@@ -0,0 +1,14 @@
+
+
+def is_valid_url(value):
+    """
+    A helper to validate a string is a URL suitable to use as root-url.
+    """
+    if not value[-1] == "/":
+        return False
+    if not value.startswith("http"):
+        return False
+    if not "://" in value:
+        return False
+
+    return True
 
=== modified file 'templates/service.conf'
--- templates/service.conf	2015-04-20 08:56:42 +0000
+++ templates/service.conf	2015-04-23 14:34:15 +0000
@@ -4,6 +4,11 @@
 # Syslog address is either a socket path or server:port string.
 syslog-address = /dev/log
 deployment-mode = {{ hosted[0]["deployment-mode"] }}
+{% if config["root-url"] %}
+root-url = {{ config["root-url"] }}
+{% else %}
+root-url = https://{{ haproxy["public-address"] }}/
+{% endif %}
 
 [stores]
 host = {{ postgres["host"] }}:{{ postgres["port"] }}