Check for physical_resource_id before getting deployments

Heat does non-strict validation of nested stack resources.
Most constraints are not enforced for nested stack resources
It's possible to set {Role}DeploymentActions as something like
[''] and it would not fail in validation. This would result in
a deployment resource with no physical_resource_id. The existing
check[1] for config-download would not be sufficient for some
cases as it can be another nested stack (we alias
both OS::Tripleo::SoftwareDeployment and
OS::Heat::StructuredDeployment).

[1]
deployment.attributes['value'].get('deployment') == \
     'TripleOSoftwareDeployment'

This adds a new check not to try and download heat deployments with
no physical_resource_id.

Change-Id: I234e629cc5377cda8d5d01bfe4416ad967cdc067
Closes-Bug: #1812604
(cherry picked from commit 54c448f683)
This commit is contained in:
Rabi Mishra 2019-01-18 22:21:03 +05:30
parent 985cb0ffd4
commit 9b3e9b2c45
2 changed files with 48 additions and 13 deletions

View File

@ -231,7 +231,12 @@ class TestConfig(base.TestCase):
return deployment_data, configs
def _get_config_dict(self, deployment):
def _get_deployment_id(self, deployment):
return deployment.attributes['value']['deployment']
def _get_config_dict(self, deployment_id):
deployment = list(filter(
lambda d: d.id == deployment_id, self.deployments))[0]
config = self.configs[deployment.attributes['value']['config']].copy()
config['inputs'] = []
config['inputs'].append(dict(
@ -247,15 +252,16 @@ class TestConfig(base.TestCase):
return yaml.safe_load(open(file_path).read())
@patch.object(ooo_config.Config, 'initialize_git_repo')
@patch('tripleo_common.utils.config.Config.get_deployment_resource_id')
@patch('tripleo_common.utils.config.Config.get_config_dict')
@patch('tripleo_common.utils.config.Config.get_deployment_data')
def test_config_download(self, mock_deployment_data, mock_config_dict,
mock_deployment_resource_id,
mock_git_init):
heat = mock.MagicMock()
self.config = ooo_config.Config(heat)
stack = mock.MagicMock()
heat.stacks.get.return_value = stack
stack.outputs = [
{'output_key': 'RoleNetHostnameMap',
'output_value': {
@ -279,8 +285,9 @@ class TestConfig(base.TestCase):
deployment_data, configs = \
self._get_config_data('config_data.yaml')
self.configs = configs
self.deployments = deployment_data
mock_deployment_data.return_value = deployment_data
mock_deployment_resource_id.side_effect = self._get_deployment_id
mock_config_dict.side_effect = self._get_config_dict
self.tmp_dir = self.useFixture(fixtures.TempDir()).path
@ -346,15 +353,17 @@ class TestConfig(base.TestCase):
d)))
@patch.object(ooo_config.Config, 'initialize_git_repo')
@patch('tripleo_common.utils.config.Config.get_deployment_resource_id')
@patch('tripleo_common.utils.config.Config.get_config_dict')
@patch('tripleo_common.utils.config.Config.get_deployment_data')
def test_config_download_os_apply_config(
self, mock_deployment_data, mock_config_dict, mock_git_init):
self, mock_deployment_data, mock_config_dict,
mock_deployment_resource_id, mock_git_init):
heat = mock.MagicMock()
self.config = ooo_config.Config(heat)
stack = mock.MagicMock()
heat.stacks.get.return_value = stack
heat.resources.get.return_value = mock.MagicMock()
stack.outputs = [
{'output_key': 'RoleNetHostnameMap',
'output_value': {
@ -397,9 +406,10 @@ class TestConfig(base.TestCase):
deployment_data.append(deployment_mock)
self.configs = configs
self.deployments = deployment_data
mock_deployment_data.return_value = deployment_data
mock_config_dict.side_effect = self._get_config_dict
mock_deployment_resource_id.side_effect = self._get_deployment_id
self.tmp_dir = self.useFixture(fixtures.TempDir()).path
with warnings.catch_warnings(record=True) as w:
@ -410,21 +420,26 @@ class TestConfig(base.TestCase):
assert "group:os-apply-config is deprecated" in str(w[-1].message)
@patch.object(ooo_config.Config, 'initialize_git_repo')
@patch('tripleo_common.utils.config.Config.get_deployment_resource_id')
@patch('tripleo_common.utils.config.Config.get_deployment_data')
def test_config_download_no_deployment_name(self, mock_deployment_data,
mock_git_init):
def test_config_download_no_deployment_name(
self, mock_deployment_data, mock_deployment_resource_id,
mock_git_init):
heat = mock.MagicMock()
self.config = ooo_config.Config(heat)
stack = mock.MagicMock()
heat.stacks.get.return_value = stack
heat.resources.get.return_value = mock.MagicMock()
deployment_data, _ = self._get_config_data('config_data.yaml')
# Delete the name of the first deployment and his parent.
del deployment_data[0].attributes['value']['name']
deployment_data[0].parent_resource = None
self.deployments = deployment_data
mock_deployment_data.return_value = deployment_data
mock_deployment_resource_id.side_effect = self._get_deployment_id
self.tmp_dir = self.useFixture(fixtures.TempDir()).path
self.assertRaises(ValueError,
@ -432,15 +447,19 @@ class TestConfig(base.TestCase):
mock_git_init.assert_called_once_with(self.tmp_dir)
@patch.object(ooo_config.Config, 'initialize_git_repo')
@patch('tripleo_common.utils.config.Config.get_deployment_resource_id')
@patch('tripleo_common.utils.config.Config.get_config_dict')
@patch('tripleo_common.utils.config.Config.get_deployment_data')
def test_config_download_no_deployment_uuid(self, mock_deployment_data,
mock_config_dict,
mock_deployment_resource_id,
mock_git_init):
heat = mock.MagicMock()
self.config = ooo_config.Config(heat)
stack = mock.MagicMock()
heat.stacks.get.return_value = stack
heat.resources.get.return_value = mock.MagicMock()
stack.outputs = [
{'output_key': 'RoleNetHostnameMap',
'output_value': {
@ -477,14 +496,20 @@ class TestConfig(base.TestCase):
deployment_data[0].attributes['value']['deployment'] = \
'TripleOSoftwareDeployment'
# Set the physical_resource_id as '' for the second deployment
deployment_data[1].attributes['value']['deployment'] = ''
self.configs = configs
self.deployments = deployment_data
mock_deployment_data.return_value = deployment_data
mock_config_dict.side_effect = self._get_config_dict
mock_deployment_resource_id.side_effect = self._get_deployment_id
self.tmp_dir = self.useFixture(fixtures.TempDir()).path
with warnings.catch_warnings(record=True) as w:
self.config.download_config(stack, self.tmp_dir)
assert "Skipping deployment" in str(w[-1].message)
assert "Skipping deployment" in str(w[-2].message)
@patch.object(ooo_config.Config, 'initialize_git_repo')
@patch.object(ooo_config.git, 'Repo')
@ -587,11 +612,11 @@ class TestConfig(base.TestCase):
deployment_data, configs = \
self._get_config_data('config_data.yaml')
self.configs = configs
self.deployments = deployment_data
stack_data = self.config.fetch_config('overcloud')
mock_deployment_data.return_value = deployment_data
mock_config_dict.side_effect = self._get_config_dict
config_dir = self.useFixture(fixtures.TempDir()).path
self.config.write_config(stack_data, 'overcloud', config_dir)

View File

@ -72,7 +72,7 @@ class Config(object):
if server_id in v:
return k
def get_config_dict(self, deployment):
def get_deployment_resource_id(self, deployment):
if '/' in deployment.attributes['value']['deployment']:
deployment_stack_id = \
deployment.attributes['value']['deployment'].split('/')[-1]
@ -82,6 +82,9 @@ class Config(object):
else:
deployment_resource_id = \
deployment.attributes['value']['deployment']
return deployment_resource_id
def get_config_dict(self, deployment_resource_id):
deployment_rsrc = self.client.software_deployments.get(
deployment_resource_id)
config = self.client.software_configs.get(
@ -261,8 +264,15 @@ class Config(object):
# create. If that's the case, just skip this deployment, otherwise
# it will result in a Not found error if we try and query the
# deployment API for this deployment.
if deployment.attributes['value'].get('deployment') == \
'TripleOSoftwareDeployment':
dep_value_resource_name = deployment.attributes[
'value'].get('deployment') == 'TripleOSoftwareDeployment'
# if not check the physical_resource_id
if not dep_value_resource_name:
deployment_resource_id = self.get_deployment_resource_id(
deployment)
if dep_value_resource_name or not deployment_resource_id:
warnings.warn('Skipping deployment %s because it has no '
'valid uuid (physical_resource_id) '
'associated.' %
@ -270,7 +280,7 @@ class Config(object):
continue
server_id = deployment.attributes['value']['server']
config_dict = self.get_config_dict(deployment)
config_dict = self.get_config_dict(deployment_resource_id)
# deployment_name should be set via the name property on the
# Deployment resources in the templates, however, if it's None