Merge "Validate the plan name before running"

tripleo_common/actions/plan.py

@@ -24,6 +24,8 @@ from swiftclient import exceptions as swiftexceptions
 from tripleo_common.actions import base
 from tripleo_common import constants
 from tripleo_common import exception
+from tripleo_common.utils.validations import pattern_validator
+
 
 LOG = logging.getLogger(__name__)
 
@@ -45,6 +47,13 @@ class CreateContainerAction(base.TripleOAction):
 
     def run(self):
         oc = self.get_object_client()
+
+        # checks to see if a container has a valid name
+        if not pattern_validator(constants.PLAN_NAME_PATTERN, self.container):
+            message = ("Unable to create plan. The plan name must "
+                       "only contain letters, numbers or dashes")
+            return mistral_workflow_utils.Result(error=message)
+
         # checks to see if a container with that name exists
         if self.container in [container["name"] for container in
                               oc.get_account()[1]]:
@@ -74,6 +83,11 @@ class CreatePlanAction(base.TripleOAction):
         env_vars = {}
         error_text = None
 
+        if not pattern_validator(constants.PLAN_NAME_PATTERN, self.container):
+            message = ("Unable to create plan. The plan name must "
+                       "only contain letters, numbers or dashes")
+            return mistral_workflow_utils.Result(error=message)
+
         # Check to see if an environment with that name already exists
         try:
             self.get_workflow_client().environments.get(self.container)

tripleo_common/constants.py

@@ -90,3 +90,5 @@ PASSWORD_PARAMETER_NAMES = (
     'TrovePassword',
     'ZaqarPassword',
 )
+
+PLAN_NAME_PATTERN = '^[a-zA-Z0-9-]+$'

tripleo_common/tests/actions/test_plan.py

@@ -86,7 +86,8 @@ class CreateContainerActionTest(base.TestCase):
 
     def setUp(self):
         super(CreateContainerActionTest, self).setUp()
-        self.container_name = 'test-container'
+        # A container that name enforces all validation rules
+        self.container_name = 'Test-container-7'
         self.expected_list = ['', [{'name': 'test1'}, {'name': 'test2'}]]
 
     @mock.patch('tripleo_common.actions.base.TripleOAction.get_object_client')
@@ -113,7 +114,7 @@ class CreateContainerActionTest(base.TestCase):
         # Setup
         swift = mock.MagicMock()
         swift.get_account.return_value = [
-            '', [{'name': 'test-container'}, {'name': 'test2'}]]
+            '', [{'name': 'Test-container-7'}, {'name': 'test2'}]]
         get_obj_client_mock.return_value = swift
 
         # Test
@@ -125,12 +126,28 @@ class CreateContainerActionTest(base.TestCase):
         self.assertEqual(result, mistral_workflow_utils.Result(
             None, error_str))
 
+    @mock.patch('tripleo_common.actions.base.TripleOAction.get_object_client')
+    def test_run_invalid_name(self, get_obj_client_mock):
+        # Setup
+        swift = mock.MagicMock()
+        get_obj_client_mock.return_value = swift
+
+        # Test
+        action = plan.CreateContainerAction("invalid_underscore")
+        result = action.run()
+
+        error_str = ("Unable to create plan. The plan name must only contain "
+                     "letters, numbers or dashes")
+        self.assertEqual(result, mistral_workflow_utils.Result(
+            None, error_str))
+
 
 class CreatePlanActionTest(base.TestCase):
 
     def setUp(self):
         super(CreatePlanActionTest, self).setUp()
-        self.container_name = 'test-container'
+        # A container that name enforces all validation rules
+        self.container_name = 'Test-container-3'
         self.capabilities_name = 'capabilities-map.yaml'
 
         # setup swift
@@ -162,7 +179,7 @@ class CreatePlanActionTest(base.TestCase):
         )
 
         self.mistral.environments.create.assert_called_once_with(
-            name='test-container',
+            name='Test-container-3',
             variables=('{"environments":'
                        ' [{"path": "/path/to/environment.yaml"}], '
                        '"template": "/path/to/overcloud.yaml"}')
@@ -190,6 +207,16 @@ class CreatePlanActionTest(base.TestCase):
         # don't bother checking the exact error (python versions different)
         self.assertEqual(result.error.split(':')[0], error_str)
 
+    def test_run_with_invalid_plan_name(self):
+
+        action = plan.CreatePlanAction("invalid_underscore")
+        result = action.run()
+
+        error_str = ("Unable to create plan. The plan name must only contain "
+                     "letters, numbers or dashes")
+        # don't bother checking the exact error (python versions different)
+        self.assertEqual(result.error.split(':')[0], error_str)
+
     def test_run_with_no_file(self):
 
         self.swift.get_object.side_effect = swiftexceptions.ClientException(

tripleo_common/tests/utils/test_validations.py

@@ -17,6 +17,7 @@ from collections import namedtuple
 import mock
 import yaml
 
+from tripleo_common.constants import PLAN_NAME_PATTERN
 from tripleo_common.tests import base
 from tripleo_common.utils import validations
 
@@ -208,3 +209,19 @@ class RunValidationTest(base.TestCase):
             'plan'
         )
         mock_find_validation.assert_called_once_with('validation')
+
+
+class RunPatternValidatorTest(base.TestCase):
+
+    def test_valid_patterns(self):
+        self.assertTrue(validations.pattern_validator("^$", ""))
+        self.assertTrue(
+            validations.pattern_validator(PLAN_NAME_PATTERN, "foo"))
+        self.assertTrue(
+            validations.pattern_validator(PLAN_NAME_PATTERN, "Foo-1"))
+
+    def test_invalid_patterns(self):
+        self.assertFalse(
+            validations.pattern_validator("^$", "foo"))
+        self.assertFalse(
+            validations.pattern_validator(PLAN_NAME_PATTERN, "foo_1"))

tripleo_common/utils/validations.py

@@ -15,6 +15,7 @@
 import glob
 import logging
 import os
+import re
 import tempfile
 import yaml
 
@@ -114,3 +115,10 @@ def cleanup_identity_file(path):
     """Write the SSH private key to disk"""
     LOG.debug('Cleaning up identity file at %s', path)
     processutils.execute('/usr/bin/sudo', '/usr/bin/rm', '-f', path)
+
+
+def pattern_validator(pattern, value):
+    LOG.debug('Validating %s with pattern %s', value, pattern)
+    if not re.match(pattern, value):
+        return False
+    return True