Merge lp:~billy-olsen/charm-helpers/better-address-endpoint-overrides into lp:charm-helpers

[Billy Olsen]

[lp:~billy-olsen/charm-helpers/better-address-endpoint-overrides]

383. By Billy Olsen on 2015-05-29

After studying the code across the OpenStack charms, it seems a better
place for the override is to place it in the canonical_url method. This
has the advantage of capturing more places besides endpoint URLs (e.g.
also for console hosts etc) and requires fewer changes to most of the
OpenStack charms.

384. By Billy Olsen on 2015-06-02

Move public address override into resolve_address.

385. By Billy Olsen on 2015-06-02

Allow configs to be None and default to http if it isn't found
for the protocol scheme

386. By Billy Olsen on 2015-06-03

Switch config option to os-public-hostname

387. By Billy Olsen on 2015-06-04

Add templating to the os-public-hostname override per jjo's bug comment.

388. By Billy Olsen on 2015-06-04

Per niedbalski's review remove jinja2 dependency and use simple string
formatting. Additionally break out the override check into its own
function.

Unmerged revisions

=== modified file 'charmhelpers/contrib/openstack/ip.py'
--- charmhelpers/contrib/openstack/ip.py	2015-02-12 22:30:09 +0000
+++ charmhelpers/contrib/openstack/ip.py	2015-06-03 18:18:45 +0000
@@ -26,8 +26,6 @@
 )
 from charmhelpers.contrib.hahelpers.cluster import is_clustered
 
-from functools import partial
-
 PUBLIC = 'public'
 INTERNAL = 'int'
 ADMIN = 'admin'
@@ -35,15 +33,18 @@
 ADDRESS_MAP = {
     PUBLIC: {
         'config': 'os-public-network',
-        'fallback': 'public-address'
+        'fallback': 'public-address',
+        'override': 'os-public-hostname',
     },
     INTERNAL: {
         'config': 'os-internal-network',
-        'fallback': 'private-address'
+        'fallback': 'private-address',
+        'override': 'os-internal-hostname',
     },
     ADMIN: {
         'config': 'os-admin-network',
-        'fallback': 'private-address'
+        'fallback': 'private-address',
+        'override': 'os-admin-hostname',
     }
 }
 
@@ -57,15 +58,30 @@
     :param endpoint_type: str endpoint type to resolve.
     :param returns: str base URL for services on the current service unit.
     """
-    scheme = 'http'
-    if 'https' in configs.complete_contexts():
-        scheme = 'https'
+    scheme = _get_scheme(configs)
+
     address = resolve_address(endpoint_type)
     if is_ipv6(address):
         address = "[{}]".format(address)
+
     return '%s://%s' % (scheme, address)
 
 
+def _get_scheme(configs):
+    """Returns the scheme to use for the url (either http or https)
+    depending upon whether https is in the configs value.
+
+    :param configs: OSTemplateRenderer config templating object to inspect
+                    for a complete https context.
+    :returns: either 'http' or 'https' depending on whether https is
+              configured within the configs context.
+    """
+    scheme = 'http'
+    if configs and 'https' in configs.complete_contexts():
+        scheme = 'https'
+    return scheme
+
+
 def resolve_address(endpoint_type=PUBLIC):
     """Return unit address depending on net config.
 
@@ -78,6 +94,14 @@
     :param endpoint_type: Network endpoing type
     """
     resolved_address = None
+
+    # Allow the user to override the address which is used. This is
+    # useful for proxy services or exposing a public endpoint url, etc.
+    override_key = ADDRESS_MAP[endpoint_type]['override']
+    addr_override = config(override_key)
+    if addr_override:
+        return addr_override
+
     vips = config('vip')
     if vips:
         vips = vips.split()
@@ -109,38 +133,3 @@
                          "clustered=%s)" % (net_type, clustered))
 
     return resolved_address
-
-
-def endpoint_url(configs, url_template, port, endpoint_type=PUBLIC,
-                 override=None):
-    """Returns the correct endpoint URL to advertise to Keystone.
-
-    This method provides the correct endpoint URL which should be advertised to
-    the keystone charm for endpoint creation. This method allows for the url to
-    be overridden to force a keystone endpoint to have specific URL for any of
-    the defined scopes (admin, internal, public).
-
-    :param configs: OSTemplateRenderer config templating object to inspect
-                    for a complete https context.
-    :param url_template: str format string for creating the url template. Only
-                         two values will be passed - the scheme+hostname
-                        returned by the canonical_url and the port.
-    :param endpoint_type: str endpoint type to resolve.
-    :param override: str the name of the config option which overrides the
-                     endpoint URL defined by the charm itself. None will
-                     disable any overrides (default).
-    """
-    if override:
-        # Return any user-defined overrides for the keystone endpoint URL.
-        user_value = config(override)
-        if user_value:
-            return user_value.strip()
-
-    return url_template % (canonical_url(configs, endpoint_type), port)
-
-
-public_endpoint = partial(endpoint_url, endpoint_type=PUBLIC)
-
-internal_endpoint = partial(endpoint_url, endpoint_type=INTERNAL)
-
-admin_endpoint = partial(endpoint_url, endpoint_type=ADMIN)

=== modified file 'tests/contrib/openstack/test_ip.py'
--- tests/contrib/openstack/test_ip.py	2015-02-12 22:30:09 +0000
+++ tests/contrib/openstack/test_ip.py	2015-06-03 18:18:45 +0000
@@ -110,31 +110,10 @@
         self.assertTrue(ip.canonical_url(configs),
                         'https://unit1')
 
-    def test_endpoint_url(self):
-        exp_url = 'https://public.example.com'
-        self.test_config.set('public-url', exp_url)
-        configs = MagicMock()
-        configs.complete_contexts.return_value = ['https']
-        endpoint = ip.endpoint_url(configs, '%s:%s', 443, ip.PUBLIC,
-                                   'public-url')
-        self.assertEqual(exp_url, endpoint)
-
-    @patch.object(ip, 'resolve_address')
-    def test_endpoint_url_no_override(self, resolve_address):
-        resolve_address.return_value = 'unit1'
-        configs = MagicMock()
-        configs.complete_contexts.return_value = ['https']
-        endpoint = ip.endpoint_url(configs, '%s:%s/v1/AUTH_$(tenant_id)s', 443,
-                                   ip.INTERNAL, 'internal-url')
-        self.assertEqual('https://unit1:443/v1/AUTH_$(tenant_id)s', endpoint)
-
-    @patch.object(ip, 'resolve_address')
-    @patch.object(ip, 'is_ipv6')
-    def test_endpoint_url_no_override_ipv6(self, is_ipv6, resolve_address):
-        resolve_address.return_value = 'unit1'
-        is_ipv6.return_value = True
-        configs = MagicMock()
-        configs.complete_contexts.return_value = ['https']
-        endpoint = ip.endpoint_url(configs, '%s:%s', 443,
-                                   ip.ADMIN, 'admin-url')
-        self.assertEquals('https://[unit1]:443', endpoint)
+    def test_canonical_url_override(self):
+        configs = MagicMock()
+        configs.complete_contexts.return_value = ['https']
+        self.test_config.set('os-public-hostname', 'public.example.com')
+        endpoint = ip.canonical_url(configs)
+        self.assertEqual('https://public.example.com',
+                         endpoint)