Validate parameters before updating

When updating parameters we save the old env try updating it
and test if that works and validates. If it doesn't the old env is
restored. The update params also now returns the flattened params
saving the ui from having to make another call after the update.

Change-Id: I9aa18c4152ff9bf896729ace0d9481af50fd7802
Closes-Bug: #1638598

tripleo_common/actions/parameters.py

@@ -13,6 +13,7 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
+import copy
 import json
 import logging
 import uuid
@@ -121,7 +122,7 @@ class ResetParametersAction(base.TripleOAction):
         return env
 
 
-class UpdateParametersAction(base.TripleOAction):
+class UpdateParametersAction(templates.ProcessTemplatesAction):
     """Updates plan environment with parameters."""
 
     def __init__(self, parameters,
@@ -134,6 +135,7 @@ class UpdateParametersAction(base.TripleOAction):
 
     def run(self, context):
         swift = self.get_object_client(context)
+        heat = self.get_orchestration_client(context)
 
         try:
             env = plan_utils.get_env(swift, self.container)
@@ -143,19 +145,71 @@ class UpdateParametersAction(base.TripleOAction):
             LOG.exception(err_msg)
             return actions.Result(error=err_msg)
 
+        saved_env = copy.deepcopy(env)
         try:
+
             plan_utils.update_in_env(swift, env, self.key,
                                      self.parameters)
+
         except swiftexceptions.ClientException as err:
             err_msg = ("Error updating environment for plan %s: %s" % (
                 self.container, err))
             LOG.exception(err_msg)
             return actions.Result(error=err_msg)
 
-        self.cache_delete(context,
+        processed_data = super(UpdateParametersAction, self).run(context)
-                          self.container,
+
-                          "tripleo.parameters.get")
+        # If we receive a 'Result' instance it is because the parent action
-        return env
+        # had an error.
+        if isinstance(processed_data, actions.Result):
+            return processed_data
+
+        processed_data['show_nested'] = True
+        env = plan_utils.get_env(swift, self.container)
+
+        params = env.get('parameter_defaults')
+        fields = {
+            'template': processed_data['template'],
+            'files': processed_data['files'],
+            'environment': processed_data['environment'],
+            'show_nested': True
+        }
+
+        try:
+            result = {
+                'heat_resource_tree': heat.stacks.validate(**fields),
+                'environment_parameters': params,
+            }
+
+            # Validation passes so the old cache gets replaced.
+            self.cache_set(context,
+                           self.container,
+                           "tripleo.parameters.get",
+                           result)
+
+            if result['heat_resource_tree']:
+                flattened = {'resources': {}, 'parameters': {}}
+                _flat_it(flattened, 'Root',
+                         result['heat_resource_tree'])
+                result['heat_resource_tree'] = flattened
+
+        except heat_exc.HTTPException as err:
+            LOG.debug("Validation failed rebuilding saved env")
+
+            # There has been an error validating we must reprocess the
+            # templates with the saved working env
+            plan_utils.put_env(swift, saved_env)
+            env = saved_env
+            processed_data = super(UpdateParametersAction, self).run(context)
+
+            err_msg = ("Error validating environment for plan %s: %s" % (
+                self.container, err))
+            LOG.exception(err_msg)
+            return actions.Result(error=err_msg)
+
+        LOG.debug("Validation worked new env is saved")
+
+        return result
 
 
 class UpdateRoleParametersAction(UpdateParametersAction):
@@ -368,47 +422,6 @@ class GetFlattenedParametersAction(GetParametersAction):
     def __init__(self, container=constants.DEFAULT_CONTAINER_NAME):
         super(GetFlattenedParametersAction, self).__init__(container)
 
-    def _process_params(self, flattened, params):
-        for item in params:
-            if item not in flattened['parameters']:
-                param_obj = {}
-                for key, value in params.get(item).items():
-                    camel_case_key = key[0].lower() + key[1:]
-                    param_obj[camel_case_key] = value
-                param_obj['name'] = item
-                flattened['parameters'][item] = param_obj
-        return list(params)
-
-    def _process(self, flattened, name, data):
-        key = str(uuid.uuid4())
-        value = {}
-        value.update({
-            'name': name,
-            'id': key
-        })
-        if 'Type' in data:
-            value['type'] = data['Type']
-        if 'Description' in data:
-            value['description'] = data['Description']
-        if 'Parameters' in data:
-            value['parameters'] = self._process_params(flattened,
-                                                       data['Parameters'])
-        if 'ParameterGroups' in data:
-            value['parameter_groups'] = data['ParameterGroups']
-        if 'NestedParameters' in data:
-            nested = data['NestedParameters']
-            nested_ids = []
-            for nested_key in nested.keys():
-                nested_data = self._process(flattened, nested_key,
-                                            nested.get(nested_key))
-                # nested_data will always have one key (and only one)
-                nested_ids.append(list(nested_data)[0])
-
-            value['resources'] = nested_ids
-
-        flattened['resources'][key] = value
-        return {key: value}
-
     def run(self, context):
         # process all plan files and create or update a stack
         processed_data = super(GetFlattenedParametersAction, self).run(context)
@@ -420,13 +433,56 @@ class GetFlattenedParametersAction(GetParametersAction):
 
         if processed_data['heat_resource_tree']:
             flattened = {'resources': {}, 'parameters': {}}
-            self._process(flattened, 'Root',
+            _flat_it(flattened, 'Root',
-                          processed_data['heat_resource_tree'])
+                     processed_data['heat_resource_tree'])
             processed_data['heat_resource_tree'] = flattened
 
         return processed_data
 
 
+def _process_params(flattened, params):
+    for item in params:
+        if item not in flattened['parameters']:
+            param_obj = {}
+            for key, value in params.get(item).items():
+                camel_case_key = key[0].lower() + key[1:]
+                param_obj[camel_case_key] = value
+            param_obj['name'] = item
+            flattened['parameters'][item] = param_obj
+    return list(params)
+
+
+def _flat_it(flattened, name, data):
+    key = str(uuid.uuid4())
+    value = {}
+    value.update({
+        'name': name,
+        'id': key
+    })
+    if 'Type' in data:
+        value['type'] = data['Type']
+    if 'Description' in data:
+        value['description'] = data['Description']
+    if 'Parameters' in data:
+        value['parameters'] = _process_params(flattened,
+                                              data['Parameters'])
+    if 'ParameterGroups' in data:
+        value['parameter_groups'] = data['ParameterGroups']
+    if 'NestedParameters' in data:
+        nested = data['NestedParameters']
+        nested_ids = []
+        for nested_key in nested.keys():
+            nested_data = _flat_it(flattened, nested_key,
+                                   nested.get(nested_key))
+            # nested_data will always have one key (and only one)
+            nested_ids.append(list(nested_data)[0])
+
+        value['resources'] = nested_ids
+
+    flattened['resources'][key] = value
+    return {key: value}
+
+
 class GetProfileOfFlavorAction(base.TripleOAction):
     """Gets the profile name for a given flavor name.
 

tripleo_common/actions/templates.py

@@ -348,8 +348,8 @@ class ProcessTemplatesAction(base.TripleOAction):
             LOG.exception("Error occurred while processing custom roles.")
             return actions.Result(error=six.text_type(err))
 
-        template_name = plan_env.get('template')
+        template_name = plan_env.get('template', "")
-        environments = plan_env.get('environments')
+        environments = plan_env.get('environments', [])
         env_paths = []
         temp_files = []
 

tripleo_common/tests/actions/test_parameters.py

@@ -255,27 +255,100 @@ class ResetParametersActionTest(base.TestCase):
 
 class UpdateParametersActionTest(base.TestCase):
 
+    @mock.patch('tripleo_common.actions.parameters.uuid')
+    @mock.patch('heatclient.common.template_utils.'
+                'process_multiple_environments_and_files')
+    @mock.patch('heatclient.common.template_utils.'
+                'get_template_contents')
     @mock.patch('tripleo_common.actions.base.TripleOAction.'
-                'cache_delete')
+                'cache_set')
-    @mock.patch('tripleo_common.actions.base.TripleOAction.get_object_client')
+    @mock.patch('tripleo_common.actions.base.TripleOAction.'
-    def test_run(self, mock_get_object_client, mock_cache):
+                'get_object_client')
+    @mock.patch('tripleo_common.actions.base.TripleOAction.'
+                'get_orchestration_client')
+    def test_run(self, mock_get_orchestration_client_client,
+                 mock_get_object_client, mock_cache,
+                 mock_get_template_contents, mock_env_files,
+                 mock_uuid):
+
+        mock_env_files.return_value = ({}, {})
 
         mock_ctx = mock.MagicMock()
 
         swift = mock.MagicMock(url="http://test.com")
+
         mock_env = yaml.safe_dump({
             'name': constants.DEFAULT_CONTAINER_NAME,
             'temp_environment': 'temp_environment',
             'template': 'template',
             'environments': [{u'path': u'environments/test.yaml'}],
         }, default_flow_style=False)
-        swift.get_object.return_value = ({}, mock_env)
+
+        mock_roles = yaml.safe_dump([{"name": "foo"}])
+        mock_network = yaml.safe_dump([{'enabled': False}])
+        mock_exclude = yaml.safe_dump({"name": "foo"})
+
+        swift.get_object.side_effect = (
+            ({}, mock_env),
+            ({}, mock_env),
+            ({}, mock_roles),
+            ({}, mock_network),
+            ({}, mock_exclude),
+            ({}, mock_env),
+            ({}, mock_env),
+            ({}, mock_env),
+            ({}, mock_roles),
+            ({}, mock_network),
+            ({}, mock_exclude),
+            ({}, mock_env),
+            ({}, mock_env),
+            swiftexceptions.ClientException('atest2')
+        )
+
+        def return_container_files(*args):
+            return ('headers', [{'name': 'foo.role.j2.yaml'}])
+
+        swift.get_container = mock.MagicMock(
+            side_effect=return_container_files)
+
         mock_get_object_client.return_value = swift
 
+        mock_heat = mock.MagicMock()
+        mock_get_orchestration_client_client.return_value = mock_heat
+
+        mock_heat.stacks.validate.return_value = {
+            "Type": "Foo",
+            "Description": "Le foo bar",
+            "Parameters": {"bar": {"foo": "bar barz"}},
+            "NestedParameters": {"Type": "foobar"}
+        }
+
+        mock_uuid.uuid4.return_value = "cheese"
+
+        expected_value = {
+            'environment_parameters': None,
+            'heat_resource_tree': {
+                'parameters': {'bar': {'foo': 'bar barz',
+                                       'name': 'bar'}},
+                'resources': {'cheese': {
+                    'id': 'cheese',
+                    'name': 'Root',
+                    'description': 'Le foo bar',
+                    'parameters': ['bar'],
+                    'resources': ['cheese'],
+                    'type': 'Foo'}
+                }
+            }
+        }
+
+        mock_get_template_contents.return_value = ({}, {
+            'heat_template_version': '2016-04-30'
+        })
+
         # Test
         test_parameters = {'SomeTestParameter': 42}
         action = parameters.UpdateParametersAction(test_parameters)
-        action.run(mock_ctx)
+        return_value = action.run(mock_ctx)
 
         mock_env_updated = yaml.safe_dump({
             'name': constants.DEFAULT_CONTAINER_NAME,
@@ -285,21 +358,42 @@ class UpdateParametersActionTest(base.TestCase):
             'environments': [{u'path': u'environments/test.yaml'}]
         }, default_flow_style=False)
 
-        swift.put_object.assert_called_once_with(
+        swift.put_object.assert_any_call(
             constants.DEFAULT_CONTAINER_NAME,
             constants.PLAN_ENVIRONMENT,
             mock_env_updated
         )
+
+        mock_heat.stacks.validate.assert_called_once_with(
+            environment={},
+            files={},
+            show_nested=True,
+            template={'heat_template_version': '2016-04-30'},
+        )
+
         mock_cache.assert_called_once_with(
             mock_ctx,
             "overcloud",
-            "tripleo.parameters.get"
+            "tripleo.parameters.get",
+            expected_value
         )
+        self.assertEqual(return_value, expected_value)
 
+    @mock.patch('heatclient.common.template_utils.'
+                'process_multiple_environments_and_files')
+    @mock.patch('heatclient.common.template_utils.'
+                'get_template_contents')
     @mock.patch('tripleo_common.actions.base.TripleOAction.'
-                'cache_delete')
+                'cache_set')
-    @mock.patch('tripleo_common.actions.base.TripleOAction.get_object_client')
+    @mock.patch('tripleo_common.actions.base.TripleOAction.'
-    def test_run_new_key(self, mock_get_object_client, mock_cache):
+                'get_object_client')
+    @mock.patch('tripleo_common.actions.base.TripleOAction.'
+                'get_orchestration_client')
+    def test_run_new_key(self, mock_get_orchestration_client_client,
+                         mock_get_object_client, mock_cache,
+                         mock_get_template_contents, mock_env_files):
+
+        mock_env_files.return_value = ({}, {})
 
         mock_ctx = mock.MagicMock()
 
@@ -310,9 +404,44 @@ class UpdateParametersActionTest(base.TestCase):
             'template': 'template',
             'environments': [{u'path': u'environments/test.yaml'}],
         }, default_flow_style=False)
-        swift.get_object.return_value = ({}, mock_env)
+
+        mock_roles = yaml.safe_dump([{"name": "foo"}])
+        mock_network = yaml.safe_dump([{'enabled': False}])
+        mock_exclude = yaml.safe_dump({"name": "foo"})
+
+        swift.get_object.side_effect = (
+            ({}, mock_env),
+            ({}, mock_env),
+            ({}, mock_roles),
+            ({}, mock_network),
+            ({}, mock_exclude),
+            ({}, mock_env),
+            ({}, mock_env),
+            ({}, mock_env),
+            ({}, mock_roles),
+            ({}, mock_network),
+            ({}, mock_exclude),
+            ({}, mock_env),
+            ({}, mock_env),
+            swiftexceptions.ClientException('atest2')
+        )
+
+        def return_container_files(*args):
+            return ('headers', [{'name': 'foo.role.j2.yaml'}])
+
+        swift.get_container = mock.MagicMock(
+            side_effect=return_container_files)
+
         mock_get_object_client.return_value = swift
 
+        heat = mock.MagicMock()
+        heat.stacks.validate.return_value = {}
+        mock_get_orchestration_client_client.return_value = heat
+
+        mock_get_template_contents.return_value = ({}, {
+            'heat_template_version': '2016-04-30'
+        })
+
         # Test
         test_parameters = {'SomeTestParameter': 42}
         action = parameters.UpdateParametersAction(test_parameters,
@@ -327,40 +456,95 @@ class UpdateParametersActionTest(base.TestCase):
             'environments': [{u'path': u'environments/test.yaml'}]
         }, default_flow_style=False)
 
-        swift.put_object.assert_called_once_with(
+        swift.put_object.assert_any_call(
             constants.DEFAULT_CONTAINER_NAME,
             constants.PLAN_ENVIRONMENT,
             mock_env_updated
         )
+
+        heat.stacks.validate.assert_called_once_with(
+            environment={},
+            files={},
+            show_nested=True,
+            template={'heat_template_version': '2016-04-30'},
+        )
+
         mock_cache.assert_called_once_with(
             mock_ctx,
             "overcloud",
-            "tripleo.parameters.get"
+            "tripleo.parameters.get",
+            {'environment_parameters': None, 'heat_resource_tree': {}}
         )
 
 
 class UpdateRoleParametersActionTest(base.TestCase):
 
+    @mock.patch('heatclient.common.template_utils.'
+                'process_multiple_environments_and_files')
+    @mock.patch('heatclient.common.template_utils.'
+                'get_template_contents')
     @mock.patch('tripleo_common.actions.base.TripleOAction.'
-                'cache_delete')
+                'cache_set')
     @mock.patch('tripleo_common.utils.parameters.set_count_and_flavor_params')
     @mock.patch('tripleo_common.actions.base.TripleOAction.'
                 'get_baremetal_client')
     @mock.patch('tripleo_common.actions.base.TripleOAction.'
                 'get_compute_client')
-    @mock.patch('tripleo_common.actions.base.TripleOAction.get_object_client')
+    @mock.patch('tripleo_common.actions.base.TripleOAction.'
-    def test_run(self, mock_get_object_client, mock_get_compute_client,
+                'get_object_client')
+    @mock.patch('tripleo_common.actions.base.TripleOAction.'
+                'get_orchestration_client')
+    def test_run(self, mock_get_orchestration_client_client,
+                 mock_get_object_client, mock_get_compute_client,
                  mock_get_baremetal_client, mock_set_count_and_flavor,
-                 mock_cache):
+                 mock_cache, mock_get_template_contents, mock_env_files):
+
+        mock_env_files.return_value = ({}, {})
 
         mock_ctx = mock.MagicMock()
 
         swift = mock.MagicMock(url="http://test.com")
-        mock_env = yaml.safe_dump({'name': 'overcast'},
+        mock_env = yaml.safe_dump({
-                                  default_flow_style=False)
+            'name': 'overcast'
-        swift.get_object.return_value = ({}, mock_env)
+        }, default_flow_style=False)
+
+        mock_roles = yaml.safe_dump([{"name": "foo"}])
+        mock_network = yaml.safe_dump([{'enabled': False}])
+        mock_exclude = yaml.safe_dump({"name": "foo"})
+
+        swift.get_object.side_effect = (
+            ({}, mock_env),
+            ({}, mock_env),
+            ({}, mock_roles),
+            ({}, mock_network),
+            ({}, mock_exclude),
+            ({}, mock_env),
+            ({}, mock_env),
+            ({}, mock_env),
+            ({}, mock_roles),
+            ({}, mock_network),
+            ({}, mock_exclude),
+            ({}, mock_env),
+            ({}, mock_env),
+            swiftexceptions.ClientException('atest2')
+        )
+
+        def return_container_files(*args):
+            return ('headers', [{'name': 'foo.yaml'}])
+
+        swift.get_container = mock.MagicMock(
+            side_effect=return_container_files)
+
         mock_get_object_client.return_value = swift
 
+        heat = mock.MagicMock()
+        heat.stacks.validate.return_value = {}
+        mock_get_orchestration_client_client.return_value = heat
+
+        mock_get_template_contents.return_value = ({}, {
+            'heat_template_version': '2016-04-30'
+        })
+
         params = {'CephStorageCount': 1,
                   'OvercloudCephStorageFlavor': 'ceph-storage'}
         mock_set_count_and_flavor.return_value = params
@@ -371,18 +555,27 @@ class UpdateRoleParametersActionTest(base.TestCase):
 
         mock_env_updated = yaml.safe_dump({
             'name': 'overcast',
-            'parameter_defaults': params,
+            'parameter_defaults': params
         }, default_flow_style=False)
 
-        swift.put_object.assert_called_once_with(
+        swift.put_object.assert_any_call(
             'overcast',
             constants.PLAN_ENVIRONMENT,
             mock_env_updated
         )
+
+        heat.stacks.validate.assert_called_once_with(
+            environment={},
+            files={},
+            show_nested=True,
+            template={'heat_template_version': '2016-04-30'},
+        )
+
         mock_cache.assert_called_once_with(
             mock_ctx,
             "overcast",
-            "tripleo.parameters.get"
+            "tripleo.parameters.get",
+            {'environment_parameters': None, 'heat_resource_tree': {}}
         )
 
 

tripleo_common/tests/actions/test_scale.py

@@ -79,6 +79,7 @@ class ScaleDownActionTest(base.TestCase):
             )
         ]
         heatclient.stacks.get.return_value = mock_stack()
+        heatclient.stacks.validate.return_value = {}
         mock_get_heat_client.return_value = heatclient
 
         mock_ctx = mock.MagicMock()
@@ -89,11 +90,31 @@ class ScaleDownActionTest(base.TestCase):
             'template': 'template',
             'environments': [{u'path': u'environments/test.yaml'}]
         }, default_flow_style=False)
+        mock_roles = yaml.safe_dump([{"name": "foo"}])
+        mock_network = yaml.safe_dump([{'enabled': False}])
+        mock_exclude = yaml.safe_dump({"name": "foo"})
         swift.get_object.side_effect = (
+            ({}, mock_env),
+            ({}, mock_env),
+            ({}, mock_roles),
+            ({}, mock_network),
+            ({}, mock_exclude),
+            ({}, mock_env),
+            ({}, mock_env),
+            ({}, mock_env),
+            ({}, mock_roles),
+            ({}, mock_network),
+            ({}, mock_exclude),
             ({}, mock_env),
             ({}, mock_env),
             swiftexceptions.ClientException('atest2')
         )
+
+        def return_container_files(*args):
+            return ('headers', [{'name': 'foo.role.j2.yaml'}])
+
+        swift.get_container = mock.MagicMock(
+            side_effect=return_container_files)
         mock_get_object_client.return_value = swift
 
         env = {
@@ -111,6 +132,13 @@ class ScaleDownActionTest(base.TestCase):
             constants.STACK_TIMEOUT_DEFAULT, ['resource_id'], 'stack')
         action.run(mock_ctx)
 
+        heatclient.stacks.validate.assert_called_once_with(
+            environment=env,
+            files={},
+            show_nested=True,
+            template={'heat_template_version': '2016-04-30'}
+        )
+
         heatclient.stacks.update.assert_called_once_with(
             'stack',
             stack_name='stack',
@@ -120,7 +148,7 @@ class ScaleDownActionTest(base.TestCase):
             files={},
             timeout_mins=240)
 
-        mock_cache.assert_called_once_with(
+        mock_cache.assert_called_with(
             mock_ctx,
             "stack",
             "tripleo.parameters.get"