Follow hostname RFCs

Updated hostname sanitization method to more closely follow RFC-952
and RFC-1123. Also moved it to nova.utils, where it seems to fit
better.

Fixes bug 885374

(Patch Set 1) Updated hostname sanitization with more efficient and
              [opinion] more readable implementation.

Change-Id: I60d7ee89867c05950bec1fd53b072a1c6247ebea

5 files changed, 91 insertions, 37 deletions

diff --git a/nova/compute/api.py b/nova/compute/api.py
index 1128da1de..0f3e0ae3d 100644
--- a/nova/compute/api.py
+++ b/nova/compute/api.py
@@ -51,34 +51,6 @@ flags.DEFINE_integer('find_host_timeout', 30,
                      'Timeout after NN seconds when looking for a host.')
 
 
-def generate_default_hostname(instance):
-    """Default function to generate a hostname given an instance reference."""
-    display_name = instance['display_name']
-    if display_name is None:
-        return 'server-%d' % (instance['id'],)
-    table = ''
-    deletions = ''
-    for i in xrange(256):
-        c = chr(i)
-        if ('a' <= c <= 'z') or ('0' <= c <= '9') or (c == '-'):
-            table += c
-        elif c in " _":
-            table += '-'
-        elif ('A' <= c <= 'Z'):
-            table += c.lower()
-        else:
-            table += '\0'
-            deletions += c
-    if isinstance(display_name, unicode):
-        display_name = display_name.encode('latin-1', 'ignore')
-    return display_name.translate(table, deletions)
-
-
-def generate_default_display_name(instance):
-    """Generate a default display name"""
-    return 'Server %s' % instance['id']
-
-
 def _is_able_to_shutdown(instance):
     vm_state = instance["vm_state"]
     instance_id = instance["id"]
@@ -115,8 +87,7 @@ def _is_queued_delete(instance):
 class API(base.Base):
     """API for interacting with the compute manager."""
 
-    def __init__(self, image_service=None, network_api=None,
-                 volume_api=None, hostname_factory=generate_default_hostname,
+    def __init__(self, image_service=None, network_api=None, volume_api=None,
                  **kwargs):
         self.image_service = image_service or \
                 nova.image.get_default_image_service()
@@ -127,7 +98,6 @@ class API(base.Base):
         if not volume_api:
             volume_api = volume.API()
         self.volume_api = volume_api
-        self.hostname_factory = hostname_factory
         super(API, self).__init__(**kwargs)
 
     def _check_injected_file_quota(self, context, injected_files):
@@ -489,17 +459,26 @@ class API(base.Base):
 
         # Set sane defaults if not specified
         updates = {}
-        if (not hasattr(instance, 'display_name') or
-                instance.display_name is None):
-            updates['display_name'] = generate_default_display_name(instance)
-            instance['display_name'] = updates['display_name']
-        updates['hostname'] = self.hostname_factory(instance)
+
+        display_name = instance.get('display_name')
+        if display_name is None:
+            display_name = self._default_display_name(instance_id)
+
+        hostname = instance.get('hostname')
+        if hostname is None:
+            hostname = display_name
+
+        updates['display_name'] = display_name
+        updates['hostname'] = utils.sanitize_hostname(hostname)
         updates['vm_state'] = vm_states.BUILDING
         updates['task_state'] = task_states.SCHEDULING
 
         instance = self.update(context, instance_id, **updates)
         return instance
 
+    def _default_display_name(self, instance_id):
+        return "Server %s" % instance_id
+
     def _schedule_run_instance(self,
             rpc_method,
             context, base_options,
@@ -783,6 +762,17 @@ class API(base.Base):
 
         :returns: None
         """
+        display_name = kwargs.get('display_name')
+        if display_name is None:
+            display_name = self._default_display_name(instance_id)
+
+        hostname = kwargs.get('hostname')
+        if hostname is None:
+            hostname = display_name
+
+        kwargs['display_name'] = display_name
+        kwargs['hostname'] = utils.sanitize_hostname(hostname)
+
         rv = self.db.instance_update(context, instance_id, kwargs)
         return dict(rv.iteritems())
 
diff --git a/nova/tests/api/openstack/test_servers.py b/nova/tests/api/openstack/test_servers.py
index aa9c5c02c..7f7e079db 100644
--- a/nova/tests/api/openstack/test_servers.py
+++ b/nova/tests/api/openstack/test_servers.py
@@ -1187,7 +1187,10 @@ class ServersControllerTest(test.TestCase):
         body = dict(server=inst_dict)
 
         def server_update(context, id, params):
-            filtered_dict = dict(display_name='server_test')
+            filtered_dict = {
+                'display_name': 'server_test',
+                'hostname': 'server-test',
+            }
             self.assertEqual(params, filtered_dict)
             return filtered_dict
 
diff --git a/nova/tests/test_compute.py b/nova/tests/test_compute.py
index 1dfc781dd..6b74a14f0 100644
--- a/nova/tests/test_compute.py
+++ b/nova/tests/test_compute.py
@@ -1237,6 +1237,30 @@ class ComputeAPITestCase(BaseTestCase):
 
         db.instance_destroy(self.context, instance_id)
 
+    def test_hostname_create(self):
+        """Ensure instance hostname is set during creation."""
+        inst_type = instance_types.get_instance_type_by_name('m1.tiny')
+        (instances, _) = self.compute_api.create(self.context,
+                                                 inst_type,
+                                                 None,
+                                                 display_name='test host')
+
+        self.assertEqual('test-host', instances[0]['hostname'])
+
+    def test_hostname_update(self):
+        """Ensure instance hostname is set during an update."""
+        instance_id = self._create_instance({"display_name": "test host"})
+        instance = db.instance_get(self.context, instance_id)
+
+        expected_hostname = 'test-host'
+        actual = self.compute_api.update(self.context,
+                                         instance_id,
+                                         **dict(instance))
+
+        self.assertEqual(expected_hostname, actual['hostname'])
+
+        db.instance_destroy(self.context, instance_id)
+
     def test_set_admin_password(self):
         """Ensure instance can have its admin password set"""
         instance_id = self._create_instance()
diff --git a/nova/tests/test_utils.py b/nova/tests/test_utils.py
index 19a15332d..2c4c02b82 100644
--- a/nova/tests/test_utils.py
+++ b/nova/tests/test_utils.py
@@ -282,6 +282,30 @@ class GenericUtilsTestCase(test.TestCase):
         result = utils.parse_server_string('www.exa:mple.com:8443')
         self.assertEqual(('', ''), result)
 
+    def test_hostname_unicode_sanitization(self):
+        hostname = u"\u7684.test.example.com"
+        self.assertEqual("test.example.com",
+                         utils.sanitize_hostname(hostname))
+
+    def test_hostname_sanitize_periods(self):
+        hostname = "....test.example.com..."
+        self.assertEqual("test.example.com",
+                         utils.sanitize_hostname(hostname))
+
+    def test_hostname_sanitize_dashes(self):
+        hostname = "----test.example.com---"
+        self.assertEqual("test.example.com",
+                         utils.sanitize_hostname(hostname))
+
+    def test_hostname_sanitize_characters(self):
+        hostname = "(#@&$!(@*--#&91)(__=+--test-host.example!!.com-0+"
+        self.assertEqual("91----test-host.example.com-0",
+                         utils.sanitize_hostname(hostname))
+
+    def test_hostname_translate(self):
+        hostname = "<}\x1fh\x10e\x08l\x02l\x05o\x12!{>"
+        self.assertEqual("hello", utils.sanitize_hostname(hostname))
+
     def test_bool_from_str(self):
         self.assertTrue(utils.bool_from_str('1'))
         self.assertTrue(utils.bool_from_str('2'))
diff --git a/nova/utils.py b/nova/utils.py
index 7723837c9..ec5de7aab 100644
--- a/nova/utils.py
+++ b/nova/utils.py
@@ -1061,3 +1061,16 @@ def total_seconds(td):
     else:
         return ((td.days * 86400 + td.seconds) * 10 ** 6 +
                 td.microseconds) / 10.0 ** 6
+
+
+def sanitize_hostname(hostname):
+    """Return a hostname which conforms to RFC-952 and RFC-1123 specs."""
+    if isinstance(hostname, unicode):
+        hostname = hostname.encode('latin-1', 'ignore')
+
+    hostname = re.sub('[ _]', '-', hostname)
+    hostname = re.sub('[^\w.-]+', '', hostname)
+    hostname = hostname.lower()
+    hostname = hostname.strip('.-')
+
+    return hostname