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. Depends-On: I8008344f215be6a54e00d7d27b697375b7f88f0f Change-Id: I8e0f2762d744b21ec1555faa1e9bbe6e2d00f67b
This commit is contained in:
Harald Jensås
parent
27d0309f3f
commit
86522255e3
|
|
@ -17,6 +17,7 @@
|
|||
import ansible_runner
|
||||
import argparse
|
||||
import datetime
|
||||
import fixtures
|
||||
import logging
|
||||
import mock
|
||||
import openstack
|
||||
|
|
@ -1992,3 +1993,61 @@ class TestGetHostEntry(base.TestCase):
|
|||
mock_process.returncode = 0
|
||||
mock_popen.return_value = mock_process
|
||||
self.assertEqual(expected, utils.get_undercloud_host_entry())
|
||||
|
||||
|
||||
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)]))
|
||||
|
|
|
|||
|
|
@ -241,6 +241,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)
|
||||
|
|
@ -286,7 +288,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()
|
||||
|
|
@ -1494,8 +1497,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([
|
||||
|
|
@ -1540,11 +1547,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]
|
||||
verifylist = [('networks_file', networks_file_path)]
|
||||
|
|
@ -1555,7 +1567,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,
|
||||
|
|
@ -1581,6 +1600,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,
|
||||
|
|
@ -1595,7 +1618,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,
|
||||
|
|
|
|||
|
|
@ -1795,6 +1795,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,
|
||||
|
|
@ -1808,17 +1824,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)
|
||||
|
|
@ -2894,3 +2901,41 @@ def create_archive_dir(archive_dir=constants.TRIPLEO_ARCHIVE_DIR):
|
|||
:return: None
|
||||
"""
|
||||
return run_command(['sudo', 'mkdir', '-p', archive_dir])
|
||||
|
||||
|
||||
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)
|
||||
|
|
|
|||
|
|
@ -259,10 +259,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 = utils.build_enabled_sevices_image_params(
|
||||
created_env_files, parsed_args, new_tht_root, user_tht_root)
|
||||
|
|
@ -290,18 +291,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
|
||||
|
||||
|
|
@ -424,8 +441,7 @@ class DeployOvercloud(command.Command):
|
|||
|
||||
utils.remove_known_hosts(overcloud_ip_or_fqdn)
|
||||
|
||||
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 []
|
||||
|
||||
|
|
@ -470,6 +486,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):
|
||||
|
|
@ -496,7 +514,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.
|
||||
|
|
@ -524,9 +542,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):
|
||||
|
|
@ -555,6 +576,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):
|
||||
|
|
@ -991,6 +1014,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):
|
||||
|
|
|
|||
Loading…
Reference in New Issue