diff --git a/tripleoclient/tests/test_utils.py b/tripleoclient/tests/test_utils.py index 659d7b9b6..73de43793 100644 --- a/tripleoclient/tests/test_utils.py +++ b/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)])) diff --git a/tripleoclient/tests/v1/overcloud_deploy/test_overcloud_deploy.py b/tripleoclient/tests/v1/overcloud_deploy/test_overcloud_deploy.py index 5e5ab104c..f0eca0e66 100644 --- a/tripleoclient/tests/v1/overcloud_deploy/test_overcloud_deploy.py +++ b/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, diff --git a/tripleoclient/utils.py b/tripleoclient/utils.py index 9ca7c37ad..bfa4d7ec7 100644 --- a/tripleoclient/utils.py +++ b/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) diff --git a/tripleoclient/v1/overcloud_deploy.py b/tripleoclient/v1/overcloud_deploy.py index 5ee69b0c1..73e818d72 100644 --- a/tripleoclient/v1/overcloud_deploy.py +++ b/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):