Merge "Handle errors when updating unknown params of plan"

tuskar/common/exception.py

@@ -201,3 +201,8 @@ class PlanExists(DuplicateEntry):
 
 class PlanAlreadyHasRole(DuplicateEntry):
     message = _("Plan %(plan_uuid)s already has role %(role_uuid)s.")
+
+
+class PlanParametersNotExist(Invalid):
+    message = _("There are no parameters named %(param_names)s"
+                " in plan %(plan_uuid)s.")

tuskar/manager/plan.py

@@ -12,6 +12,7 @@
 
 import logging
 
+from tuskar.common import exception
 from tuskar.manager import models
 from tuskar.manager import name_utils
 from tuskar.storage.exceptions import UnknownName
@@ -291,9 +292,24 @@ class PlansManager(object):
             db_plan.environment_file.contents
         )
 
+        non_existent_params = []
         for param_value in params:
             p = environment.find_parameter_by_name(param_value.name)
-            p.value = param_value.value
+            if p:
+                p.value = param_value.value
+            else:
+                non_existent_params.append(param_value.name)
+
+        if non_existent_params:
+            param_names = ', '.join(non_existent_params)
+            LOG.error(
+                'There are no parameters named %(param_names)s'
+                ' in plan %(plan_uuid)s.' %
+                {'param_names': param_names, 'plan_uuid': plan_uuid})
+            raise exception.PlanParametersNotExist(
+                plan_uuid=plan_uuid,
+                param_names=param_names
+            )
 
         # Save the updated environment.
         env_contents = composer.compose_environment(environment)

tuskar/tests/manager/test_plan.py

@@ -10,6 +10,7 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
+from tuskar.common import exception
 from tuskar.manager.models import DeploymentPlan
 from tuskar.manager.models import ParameterValue
 from tuskar.manager.models import PlanParameter
@@ -319,6 +320,48 @@ class PlansManagerTestCase(TestCase):
         self.assertEqual(found_params[2].value, 'm1.small')
         self.assertEqual(found_params[3].value, 'test-key')
 
+    def test_set_non_existent_parameters(self):
+        # Setup
+        test_role = self._add_test_role()
+        test_plan = self.plans_manager.create_plan('p1', 'd1')
+        self.plans_manager.add_role_to_plan(test_plan.uuid, test_role.uuid)
+
+        # Test
+        ns = name_utils.generate_role_namespace(test_role.name,
+                                                test_role.version)
+        not_present_in_role_1_name = ns_utils.apply_template_namespace(
+            ns, 'not_present_in_role_1')
+        not_present_in_role_2_name = ns_utils.apply_template_namespace(
+            ns, 'not_present_in_role_2')
+        update_us = [
+            ParameterValue(ns_utils.apply_template_namespace(ns, 'key_name'),
+                           'test-key'),
+            ParameterValue(ns_utils.apply_template_namespace(ns, 'image_id'),
+                           'test-image'),
+            ParameterValue(not_present_in_role_1_name,
+                           'not-present-in-role-1-value'),
+            ParameterValue(not_present_in_role_2_name,
+                           'not-present-in-role-2-value'),
+        ]
+
+        # Verify
+        exc = self.assertRaises(exception.PlanParametersNotExist,
+                                self.plans_manager.set_parameter_values,
+                                test_plan.uuid,
+                                update_us)
+        self.assertIn(not_present_in_role_1_name, str(exc))
+        self.assertIn(not_present_in_role_2_name, str(exc))
+
+        # Pull it from the database to make sure it was modified
+        found = self.plans_manager.retrieve_plan(test_plan.uuid)
+        found_params = sorted(found.parameters, key=lambda x: x.name)
+        self.assertEqual(4, len(found_params))  # 3 + 1 for scaling
+        self.assertEqual(found_params[0].value, '1')
+        self.assertEqual(found_params[1].value,
+                         '3e6270da-fbf7-4aef-bc78-6d0cfc3ad11b')
+        self.assertEqual(found_params[2].value, 'm1.small')
+        self.assertEqual(found_params[3].value, '')
+
     def test_package_templates(self):
         # Setup
         test_role = self._add_test_role()