diff --git a/ironic/common/utils.py b/ironic/common/utils.py index 6d5c1b0a17..3633f826c4 100644 --- a/ironic/common/utils.py +++ b/ironic/common/utils.py @@ -184,16 +184,26 @@ def is_hostname_safe(hostname): * http://en.wikipedia.org/wiki/Hostname * http://tools.ietf.org/html/rfc952 * http://tools.ietf.org/html/rfc1123 - - Also allow "." because what kind of hostname doesn't allow that. + Allowing for hostnames, and hostnames + domains. :param hostname: The hostname to be validated. :returns: True if valid. False if not. """ - m = '^[a-z0-9]([a-z0-9\-\.]{0,61}[a-z0-9])?$' - return (isinstance(hostname, six.string_types) and - (re.match(m, hostname) is not None)) + if not isinstance(hostname, six.string_types) or len(hostname) > 255: + return False + + # Periods on the end of a hostname are ok, but complicates the + # regex so we'll do this manually + if hostname.endswith('.'): + hostname = hostname[:-1] + + host = '[a-z0-9]([a-z0-9\-]{0,61}[a-z0-9])?' + domain = '[a-z0-9\-_]{0,62}[a-z0-9]' + + m = '^' + host + '(\.' + domain + ')*$' + + return re.match(m, hostname) is not None def validate_and_normalize_mac(address): diff --git a/ironic/db/sqlalchemy/alembic/versions/2fb93ffd2af1_increase_node_name_length.py b/ironic/db/sqlalchemy/alembic/versions/2fb93ffd2af1_increase_node_name_length.py new file mode 100644 index 0000000000..e690d61685 --- /dev/null +++ b/ironic/db/sqlalchemy/alembic/versions/2fb93ffd2af1_increase_node_name_length.py @@ -0,0 +1,42 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +"""increase-node-name-length + +Revision ID: 2fb93ffd2af1 +Revises: 4f399b21ae71 +Create Date: 2015-03-18 17:08:11.470791 + +""" + +# revision identifiers, used by Alembic. +revision = '2fb93ffd2af1' +down_revision = '4f399b21ae71' + + +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import mysql + + +def upgrade(): + op.alter_column('nodes', 'name', + existing_type=mysql.VARCHAR(length=63), + type_=sa.String(length=255), + existing_nullable=True) + + +def downgrade(): + op.alter_column('nodes', 'name', + existing_type=sa.String(length=255), + type_=mysql.VARCHAR(length=63), + existing_nullable=True) diff --git a/ironic/db/sqlalchemy/models.py b/ironic/db/sqlalchemy/models.py index ecc908885e..f9c67317ad 100644 --- a/ironic/db/sqlalchemy/models.py +++ b/ironic/db/sqlalchemy/models.py @@ -154,7 +154,7 @@ class Node(Base): # filter on it more efficiently, even though it is # user-settable, and would otherwise be in node.properties. instance_uuid = Column(String(36), nullable=True) - name = Column(String(63), nullable=True) + name = Column(String(255), nullable=True) chassis_id = Column(Integer, ForeignKey('chassis.id'), nullable=True) power_state = Column(String(15), nullable=True) target_power_state = Column(String(15), nullable=True) diff --git a/ironic/tests/db/sqlalchemy/test_migrations.py b/ironic/tests/db/sqlalchemy/test_migrations.py index b6ba0e0956..70fb4cabd0 100644 --- a/ironic/tests/db/sqlalchemy/test_migrations.py +++ b/ironic/tests/db/sqlalchemy/test_migrations.py @@ -374,6 +374,12 @@ class MigrationCheckersMixin(object): self.assertIsInstance(nodes.c.clean_step.type, sqlalchemy.types.String) + def _check_2fb93ffd2af1(self, engine, data): + # TODO(mrda): Currently database migration tests aren't running + # But once that has been resolved, add a new db migration test here + # for increasing node name length. See bug 1438531 + pass + def test_upgrade_and_version(self): with patch_with_engine(self.engine): self.migration_api.upgrade('head') diff --git a/ironic/tests/test_utils.py b/ironic/tests/test_utils.py index 044f32beea..5e205bcfcc 100644 --- a/ironic/tests/test_utils.py +++ b/ironic/tests/test_utils.py @@ -342,6 +342,7 @@ class GenericUtilsTestCase(base.TestCase): self.assertFalse(utils.is_hostname_safe('-spam')) self.assertFalse(utils.is_hostname_safe('spam-')) self.assertTrue(utils.is_hostname_safe('spam-eggs')) + self.assertFalse(utils.is_hostname_safe('spam_eggs')) self.assertFalse(utils.is_hostname_safe('spam eggs')) self.assertTrue(utils.is_hostname_safe('spam.eggs')) self.assertTrue(utils.is_hostname_safe('9spam')) @@ -360,6 +361,18 @@ class GenericUtilsTestCase(base.TestCase): # Need to ensure a binary response for success or fail self.assertIsNotNone(utils.is_hostname_safe('spam')) self.assertIsNotNone(utils.is_hostname_safe('-spam')) + self.assertTrue(utils.is_hostname_safe('www.rackspace.com')) + self.assertTrue(utils.is_hostname_safe('www.rackspace.com.')) + self.assertTrue(utils.is_hostname_safe('http._sctp.www.example.com')) + self.assertTrue(utils.is_hostname_safe('mail.pets_r_us.net')) + self.assertTrue(utils.is_hostname_safe('mail-server-15.my_host.org')) + self.assertFalse(utils.is_hostname_safe('www.nothere.com_')) + self.assertFalse(utils.is_hostname_safe('www.nothere_.com')) + self.assertFalse(utils.is_hostname_safe('www..nothere.com')) + long_str = 'a' * 63 + '.' + 'b' * 63 + '.' + 'c' * 63 + '.' + 'd' * 63 + self.assertTrue(utils.is_hostname_safe(long_str)) + self.assertFalse(utils.is_hostname_safe(long_str + '.')) + self.assertFalse(utils.is_hostname_safe('a' * 255)) def test_validate_and_normalize_mac(self): mac = 'AA:BB:CC:DD:EE:FF'