Implement protected resource types

When using the oc deploy command with the options
--baremetal-provision, --vip-file and network-v2
version networks definition the heat environment
files are added to user environments internally.
Including the legacy environment files, such as
`network-isolation.yaml` would result in overriding
the internally defined resource types and a failed
deployment.

This patch implements a check that will detect
resource type conflicts and raise error if a
protected resource type is overriden.

For users that may still want/need to override the
resource types, the protection can be disabled by
setting: `--disable-protected-resource-types`

NOTE: Parameter's are left unprotected since
  traditionally THT interfaces always allowed
  overriding anything and everything.

Also refactor the process_multiple_environments
method by splitting part of method to the new
rewrite_env_path method, so that it can be used
also when checking for prohibited overrides.

Conflicts:
  tripleoclient/tests/test_utils.py
  tripleoclient/utils.py
  tripleoclient/v1/overcloud_deploy.py

Depends-On: I8008344f215be6a54e00d7d27b697375b7f88f0f
Change-Id: I8e0f2762d744b21ec1555faa1e9bbe6e2d00f67b
(cherry picked from commit 86522255e3)

tripleoclient/tests/test_utils.py

@@ -17,6 +17,7 @@
 import ansible_runner
 import argparse
 import datetime
+import fixtures
 import logging
 import mock
 import os
@@ -1870,3 +1871,61 @@ class TestGeneralUtils(base.TestCommand):
         limit_hosts_expected = 'controller0:compute0:compute1:!compute2'
         limit_hosts_actual = utils.playbook_limit_parse(limit_nodes)
         self.assertEqual(limit_hosts_actual, limit_hosts_expected)
+
+
+class TestProhibitedOverrides(base.TestCommand):
+
+    def setUp(self):
+        super(TestProhibitedOverrides, self).setUp()
+        self.tmp_dir = self.useFixture(fixtures.TempDir())
+
+    def test_extend_protected_overrides(self):
+        protected_overrides = {
+            'registry_entries': {'OS::Foo::Bar': ['foo_bar_file']}}
+        output_path = self.tmp_dir.join('env-file.yaml')
+        fake_env = {
+            'parameter_defaults': {
+                'DeployedNetworkEnvironment': {'foo': 'bar'}},
+            'resource_registry': {
+                'OS::TripleO::Network': 'foo'}
+        }
+        with open(output_path, 'w') as temp_file:
+            yaml.safe_dump(fake_env, temp_file)
+
+        utils.extend_protected_overrides(protected_overrides, output_path)
+        self.assertEqual({
+            'registry_entries': {
+                'OS::Foo::Bar': ['foo_bar_file'],
+                'OS::TripleO::Network': [output_path]}},
+            protected_overrides)
+
+    def test_check_prohibited_overrides_with_conflict(self):
+        protected_overrides = {
+            'registry_entries': {'OS::Foo::Bar': ['foo_bar_file']}}
+        user_env = self.tmp_dir.join('env-file01.yaml')
+        fake_env = {'parameter_defaults': {'foo_param': {'foo': 'bar'}},
+                    'resource_registry': {'OS::Foo::Bar': 'foo'}}
+        with open(user_env, 'w') as temp_file:
+            yaml.safe_dump(fake_env, temp_file)
+
+        self.assertRaises(exceptions.DeploymentError,
+                          utils.check_prohibited_overrides,
+                          protected_overrides, [(user_env, user_env)])
+        self.assertRaisesRegex(
+            exceptions.DeploymentError,
+            'ERROR: Protected resource registry overrides detected!',
+            utils.check_prohibited_overrides,
+            protected_overrides, [(user_env, user_env)])
+
+    def test_check_prohibited_overrides_with_no_conflict(self):
+        protected_overrides = {
+            'registry_entries': {'OS::Foo::Bar': ['foo_bar_file']}}
+        user_env = self.tmp_dir.join('env-file01.yaml')
+        fake_env = {'parameter_defaults': {'bar_param': {'bar': 'foo'}},
+                    'resource_registry': {'OS::Bar::Foo': 'bar'}}
+        with open(user_env, 'w') as temp_file:
+            yaml.safe_dump(fake_env, temp_file)
+
+        self.assertIsNone(
+            utils.check_prohibited_overrides(protected_overrides,
+                                             [(user_env, user_env)]))

tripleoclient/tests/v1/overcloud_deploy/test_overcloud_deploy.py

@@ -236,6 +236,8 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud):
             output_dir=self.cmd.working_dir)
         mock_copy.assert_called_once()
 
+    @mock.patch('tripleoclient.v1.overcloud_deploy.DeployOvercloud.'
+                '_provision_virtual_ips', autospec=True)
     @mock.patch('tripleoclient.v1.overcloud_deploy.DeployOvercloud.'
                 '_provision_networks', autospec=True)
     @mock.patch('tripleoclient.utils.build_stack_data', autospec=True)
@@ -283,7 +285,8 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud):
                         mock_process_env, mock_roles_data,
                         mock_container_prepare, mock_generate_password,
                         mock_rc_params, mock_default_image_params,
-                        mock_stack_data, mock_provision_networks):
+                        mock_stack_data, mock_provision_networks,
+                        mock_provision_virtual_ips):
         fixture = deployment.DeploymentWorkflowFixture()
         self.useFixture(fixture)
         utils_fixture = deployment.UtilsFixture()
@@ -1508,8 +1511,12 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud):
         with open(env_path, 'w') as f:
             yaml.safe_dump(baremetal_deployed, f)
 
+        protected_overrides = {'registry_entries': dict(),
+                               'parameter_entries': dict()}
+
         self.cmd.working_dir = self.tmp_dir.join('working_dir')
-        result = self.cmd._provision_baremetal(parsed_args, tht_root)
+        result = self.cmd._provision_baremetal(parsed_args, tht_root,
+                                               protected_overrides)
         self.cmd._unprovision_baremetal(parsed_args)
         self.assertEqual([env_path], result)
         self.mock_playbook.assert_has_calls([
@@ -1555,11 +1562,16 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud):
 
     def test__provision_networks(self):
         networks_file_path = self.tmp_dir.join('networks.yaml')
-        network_data = [
-            {'name': 'Network', 'name_lower': 'network', 'subnets': {}}
-        ]
+        fake_network_data = [{'name': 'Network', 'name_lower': 'network'}]
+        fake_deployed_env = {
+            'parameter_defaults':
+                {'DeployedNetworkEnvironment': {'foo': 'bar'}},
+            'resource_registry':
+                {'OS::TripleO::Network': 'foo'}
+        }
+
         with open(networks_file_path, 'w') as temp_file:
-            yaml.safe_dump(network_data, temp_file)
+            yaml.safe_dump(fake_network_data, temp_file)
 
         arglist = ['--networks-file', networks_file_path,
                    '--templates', constants.TRIPLEO_HEAT_TEMPLATES]
@@ -1572,7 +1584,14 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud):
         env_path = os.path.join(env_dir, 'networks-deployed.yaml')
         os.makedirs(env_dir)
 
-        result = self.cmd._provision_networks(parsed_args, tht_root)
+        with open(env_path, 'w') as env_file:
+            env_file.write(yaml.safe_dump(data=fake_deployed_env))
+
+        protected_overrides = {'registry_entries': dict(),
+                               'parameter_entries': dict()}
+
+        result = self.cmd._provision_networks(parsed_args, tht_root,
+                                              protected_overrides)
         self.assertEqual([env_path], result)
         self.mock_playbook.assert_called_once_with(
             extra_vars={'network_data_path': networks_file_path,
@@ -1599,6 +1618,10 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud):
         with open(vips_file_path, 'w') as temp_file:
             yaml.safe_dump(vip_data, temp_file)
 
+        fake_deployed_env = {
+            'parameter_defaults': {'VipPortMap': {'external': {'foo': 'bar'}}},
+            'resource_registry': {
+                'OS::TripleO::Network::Ports::ExternalVipPort': 'foo'}}
         stack_name = 'overcloud'
         arglist = ['--stack', stack_name,
                    '--vip-file', vips_file_path,
@@ -1615,7 +1638,14 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud):
         env_path = os.path.join(env_dir, 'virtual-ips-deployed.yaml')
         os.makedirs(env_dir)
 
-        result = self.cmd._provision_virtual_ips(parsed_args, tht_root)
+        with open(env_path, 'w') as env_file:
+            env_file.write(yaml.safe_dump(data=fake_deployed_env))
+
+        protected_overrides = {'registry_entries': dict(),
+                               'parameter_entries': dict()}
+
+        result = self.cmd._provision_virtual_ips(parsed_args, tht_root,
+                                                 protected_overrides)
         self.assertEqual([env_path], result)
         self.mock_playbook.assert_called_once_with(
             extra_vars={'stack_name': stack_name,

tripleoclient/utils.py

@@ -1785,6 +1785,22 @@ def jinja_render_files(log, templates, working_dir,
         raise exceptions.DeploymentError(msg)
 
 
+def rewrite_env_path(env_path, tht_root, user_tht_root, log=None):
+    abs_env_path = os.path.abspath(env_path)
+    if (abs_env_path.startswith(user_tht_root)
+            and ((user_tht_root + '/') in env_path
+                 or (user_tht_root + '/') in abs_env_path
+                 or user_tht_root == abs_env_path
+                 or user_tht_root == env_path)):
+        new_env_path = env_path.replace(user_tht_root + '/', tht_root + '/')
+        if log:
+            log.debug("Redirecting env file %s to %s"
+                      % (abs_env_path, new_env_path))
+        env_path = new_env_path
+
+    return env_path, abs_env_path
+
+
 def process_multiple_environments(created_env_files, tht_root,
                                   user_tht_root,
                                   env_files_tracker=None,
@@ -1798,17 +1814,8 @@ def process_multiple_environments(created_env_files, tht_root,
     tht_root = os.path.normpath(tht_root)
     for env_path in created_env_files:
         log.debug("Processing environment files %s" % env_path)
-        abs_env_path = os.path.abspath(env_path)
-        if (abs_env_path.startswith(user_tht_root) and
-            ((user_tht_root + '/') in env_path or
-             (user_tht_root + '/') in abs_env_path or
-             user_tht_root == abs_env_path or
-             user_tht_root == env_path)):
-            new_env_path = env_path.replace(user_tht_root + '/',
-                                            tht_root + '/')
-            log.debug("Redirecting env file %s to %s"
-                      % (abs_env_path, new_env_path))
-            env_path = new_env_path
+        env_path, abs_env_path = rewrite_env_path(env_path, tht_root,
+                                                  user_tht_root, log=log)
         try:
             files, env = template_utils.process_environment_and_files(
                 env_path=env_path, include_env_in_files=include_env_in_files)
@@ -2740,3 +2747,41 @@ def run_role_playbooks(self, working_dir, roles_file_dir, roles):
     # Network Config
     run_role_playbook(self, inventory, constants.ANSIBLE_TRIPLEO_PLAYBOOKS,
                       'cli-overcloud-node-network-config.yaml')
+
+
+def extend_protected_overrides(protected_overrides, output_path):
+    with open(output_path, 'r') as env_file:
+        data = yaml.safe_load(env_file.read())
+
+    protect_registry = protected_overrides['registry_entries']
+    resource_registry = data.get('resource_registry', {})
+
+    for reg_entry in resource_registry.keys():
+        protect_registry.setdefault(reg_entry, []).append(output_path)
+
+
+def check_prohibited_overrides(protected_overrides, user_environments):
+    found_conflict = False
+    protected_registry = protected_overrides['registry_entries']
+    msg = ("ERROR: Protected resource registry overrides detected! These "
+           "entries are used in internal environments and should not be "
+           "overridden in the user environment. Please remove these overrides "
+           "from the environment files.\n")
+    for env_path, abs_env_path in user_environments:
+        with open(env_path, 'r') as file:
+            data = yaml.safe_load(file.read())
+        registry = set(data.get('resource_registry', {}).keys())
+
+        conflicts = set(protected_registry.keys()).intersection(registry)
+        if not conflicts:
+            continue
+
+        found_conflict = True
+        for x in conflicts:
+            msg += ("Conflict detected for resource_registry entry: {}.\n"
+                    "\tUser environment: {}.\n"
+                    "\tInternal environment: {}\n").format(
+                x, abs_env_path, protected_registry[x])
+
+    if found_conflict:
+        raise exceptions.DeploymentError(msg)

tripleoclient/v1/overcloud_deploy.py

@@ -278,10 +278,11 @@ class DeployOvercloud(command.Command):
     def create_env_files(self, stack, parsed_args,
                          new_tht_root, user_tht_root):
         self.log.debug("Creating Environment files")
-        created_env_files = []
-
-        created_env_files.append(
-            os.path.join(new_tht_root, constants.DEFAULT_RESOURCE_REGISTRY))
+        # A dictionary to store resource registry types that are internal,
+        # and should not be overridden in user provided environments.
+        protected_overrides = {'registry_entries': dict()}
+        created_env_files = [
+            os.path.join(new_tht_root, constants.DEFAULT_RESOURCE_REGISTRY)]
 
         parameters = self.build_image_params(
             created_env_files, parsed_args, new_tht_root, user_tht_root)
@@ -309,18 +310,34 @@ class DeployOvercloud(command.Command):
 
         if not parsed_args.skip_nodes_and_networks:
             created_env_files.extend(
-                self._provision_networks(parsed_args, new_tht_root))
+                self._provision_networks(parsed_args, new_tht_root,
+                                         protected_overrides))
             created_env_files.extend(
-                self._provision_virtual_ips(parsed_args, new_tht_root))
+                self._provision_virtual_ips(parsed_args, new_tht_root,
+                                            protected_overrides))
             created_env_files.extend(
-                self._provision_baremetal(parsed_args, new_tht_root))
+                self._provision_baremetal(parsed_args, new_tht_root,
+                                          protected_overrides))
 
+        user_environments = []
         if parsed_args.environment_directories:
-            created_env_files.extend(utils.load_environment_directories(
+            user_environments.extend(utils.load_environment_directories(
                 parsed_args.environment_directories))
 
         if parsed_args.environment_files:
-            created_env_files.extend(parsed_args.environment_files)
+            user_environments.extend(parsed_args.environment_files)
+
+        if (not parsed_args.disable_protected_resource_types
+                and user_environments):
+            rewritten_user_environments = []
+            for env_path in user_environments:
+                env_path, abs_env_path = utils.rewrite_env_path(
+                    env_path, new_tht_root, user_tht_root)
+                rewritten_user_environments.append((env_path, abs_env_path))
+            utils.check_prohibited_overrides(protected_overrides,
+                                             rewritten_user_environments)
+
+        created_env_files.extend(user_environments)
 
         return created_env_files
 
@@ -564,8 +581,7 @@ class DeployOvercloud(command.Command):
                 "Error: The following environment directories were not found"
                 ": {0}".format(", ".join(nonexisting_dirs)))
 
-    def _provision_baremetal(self, parsed_args, tht_root):
-
+    def _provision_baremetal(self, parsed_args, tht_root, protected_overrides):
         if not parsed_args.baremetal_deployment:
             return []
 
@@ -611,6 +627,8 @@ class DeployOvercloud(command.Command):
             utils.run_role_playbooks(self, self.working_dir, roles_file_dir,
                                      roles)
 
+        utils.extend_protected_overrides(protected_overrides, output_path)
+
         return [output_path]
 
     def _unprovision_baremetal(self, parsed_args):
@@ -637,7 +655,7 @@ class DeployOvercloud(command.Command):
                 }
             )
 
-    def _provision_networks(self, parsed_args, tht_root):
+    def _provision_networks(self, parsed_args, tht_root, protected_overrides):
         # Parse the network data, if any network have 'ip_subnet' or
         # 'ipv6_subnet' keys this is not a network-v2 format file. In this
         # case do nothing.
@@ -666,9 +684,12 @@ class DeployOvercloud(command.Command):
                 extra_vars=extra_vars,
             )
 
+        utils.extend_protected_overrides(protected_overrides, output_path)
+
         return [output_path]
 
-    def _provision_virtual_ips(self, parsed_args, tht_root):
+    def _provision_virtual_ips(self, parsed_args, tht_root,
+                               protected_overrides):
         networks_file_path = utils.get_networks_file_path(
             parsed_args.networks_file, parsed_args.templates)
         if not utils.is_network_data_v2(networks_file_path):
@@ -698,6 +719,8 @@ class DeployOvercloud(command.Command):
                 extra_vars=extra_vars,
             )
 
+        utils.extend_protected_overrides(protected_overrides, output_path)
+
         return [output_path]
 
     def setup_ephemeral_heat(self, parsed_args):
@@ -1134,6 +1157,17 @@ class DeployOvercloud(command.Command):
                    'the necessary input for the stack create must be provided '
                    'by the user.')
         )
+        parser.add_argument(
+            '--disable-protected-resource-types',
+            action='store_true',
+            default=False,
+            help=_('Disable protected resource type overrides. Resources '
+                   'types that are used internally are protected, and cannot '
+                   'be overridden in the user environment. Setting this '
+                   'argument disables the protection, allowing the protected '
+                   'resource types to be override in the user environment.')
+        )
+
         return parser
 
     def take_action(self, parsed_args):