openstack
/
heat
Code Issues Proposed changes

Fix filter name inconsistency in stack_list 

As described in the bug report, filter names used in stack_list API are
not consistent with what users see when creating or retrieving stacks.
We cannot simply change the exposed filter key names because that will
break existing users.

This patch adds a translation logic from STACK_KEYS like 'stack_name' to
its corresponding db column name, i.e. 'name'. The intent is to provide
compatibility with existing use cases while enforcing API consistency
regarding the "properties" a user sees from the API.

At engine API layer, we also try a simple parsing of the 'status' key,
which may be of simple form 'FAILED' or complex form 'CREATE_COMPLETE'.

Change-Id: Ia6fe4345ca9feaf20c3bd020ff454e5d701a458e
Closes-Bug: 1461838
This commit is contained in:
tengqm 2015-06-04 10:09:28 -04:00
parent cd10238cce
commit b910a27d5f
6 changed files with 285 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
heat/api/openstack/v1/stacks.py
View File

@ -186,6 +186,8 @@ class StackController(object):
def _index(self, req, tenant_safe=True):
filter_whitelist = {
# usage of keys in this list are not encouraged, please use
# rpc_api.STACK_KEYS instead
'id': 'mixed',
'status': 'mixed',
'name': 'mixed',
@ -208,7 +210,25 @@ class StackController(object):
'not_tags_any': 'single',
}
params = util.get_allowed_params(req.params, whitelist)
filter_params = util.get_allowed_params(req.params, filter_whitelist)
stack_keys = dict.fromkeys(rpc_api.STACK_KEYS, 'mixed')
unsupported = (
rpc_api.STACK_ID, # not user visible
rpc_api.STACK_CAPABILITIES, # not supported
rpc_api.STACK_CREATION_TIME, # don't support timestamp
rpc_api.STACK_DELETION_TIME, # don't support timestamp
rpc_api.STACK_DESCRIPTION, # not supported
rpc_api.STACK_NOTIFICATION_TOPICS, # not supported
rpc_api.STACK_OUTPUTS, # not in database
rpc_api.STACK_PARAMETERS, # not in this table
rpc_api.STACK_TAGS, # tags query following a specific guideline
rpc_api.STACK_TMPL_DESCRIPTION, # not supported
rpc_api.STACK_UPDATED_TIME, # don't support timestamp
)
for key in unsupported:
stack_keys.pop(key)
# downward compatibility
stack_keys.update(filter_whitelist)
filter_params = util.get_allowed_params(req.params, stack_keys)
show_deleted = False
p_name = rpc_api.PARAM_SHOW_DELETED

84
heat/engine/api.py
View File

@ -86,6 +86,90 @@ def extract_args(params):
return kwargs
def _parse_object_status(status):
"""Parse input status into action and status if possible.
This function parses a given string (or list of strings) and see if it
contains the action part. The action part is exacted if found.
:param status: A string or a list of strings where each string contains
a status to be checked.
:returns: (actions, statuses) tuple, where actions is a set of actions
extracted from the input status and statuses is a set of pure
object status.
"""
if not isinstance(status, list):
status = [status]
status_set = set()
action_set = set()
for val in status:
# Note: cannot reference Stack.STATUSES due to circular reference issue
for s in ('COMPLETE', 'FAILED', 'IN_PROGRESS'):
index = val.rfind(s)
if index != -1:
status_set.add(val[index:])
if index > 1:
action_set.add(val[:index - 1])
break
return action_set, status_set
def translate_filters(params):
"""Translate filter names to their corresponding DB field names.
:param params: A dictionary containing keys from engine.api.STACK_KEYS
and other keys previously leaked to users.
:returns: A dict containing only valid DB filed names.
"""
key_map = {
rpc_api.STACK_NAME: 'name',
rpc_api.STACK_ACTION: 'action',
rpc_api.STACK_STATUS: 'status',
rpc_api.STACK_STATUS_DATA: 'status_reason',
rpc_api.STACK_DISABLE_ROLLBACK: 'disable_rollback',
rpc_api.STACK_TIMEOUT: 'timeout',
rpc_api.STACK_OWNER: 'username',
rpc_api.STACK_PARENT: 'owner_id',
rpc_api.STACK_USER_PROJECT_ID: 'stack_user_project_id',
}
for key, field in key_map.items():
value = params.pop(key, None)
if not value:
continue
fld_value = params.get(field, None)
if fld_value:
if not isinstance(fld_value, list):
fld_value = [fld_value]
if not isinstance(value, list):
value = [value]
value.extend(fld_value)
params[field] = value
# Deal with status which might be of form <ACTION>_<STATUS>, e.g.
# "CREATE_FAILED". Note this logic is still not ideal due to the fact
# that action and status are stored separately.
if 'status' in params:
a_set, s_set = _parse_object_status(params['status'])
statuses = sorted(s_set)
params['status'] = statuses[0] if len(statuses) == 1 else statuses
if a_set:
a = params.get('action', [])
action_set = set(a) if isinstance(a, list) else set([a])
actions = sorted(action_set.union(a_set))
params['action'] = actions[0] if len(actions) == 1 else actions
return params
def format_stack_outputs(stack, outputs):
'''
Return a representation of the given output template for the given stack

3
heat/engine/service.py
View File

@ -500,6 +500,9 @@ class EngineService(service.Service):
multiple tags using the boolean OR expression
:returns: a list of formatted stacks
"""
if filters is not None:
filters = api.translate_filters(filters)
stacks = parser.Stack.load_all(cnxt, limit, marker, sort_keys,
sort_dir, filters, tenant_safe,
show_deleted, resolve_data=False,

43
heat/tests/api/openstack_v1/test_stacks.py
View File

@ -321,7 +321,27 @@ class StackControllerTest(tools.ControllerTest, common.HeatTestCase):
'username': 'fake username',
'tenant': 'fake tenant',
'owner_id': 'fake owner-id',
'balrog': 'you shall not pass!'
'stack_name': 'fake stack name',
'stack_identity': 'fake identity',
'creation_time': 'create timestamp',
'updated_time': 'update timestamp',
'deletion_time': 'deletion timestamp',
'notification_topics': 'fake topic',
'description': 'fake description',
'template_description': 'fake description',
'parameters': 'fake params',
'outputs': 'fake outputs',
'stack_action': 'fake action',
'stack_status': 'fake status',
'stack_status_reason': 'fake status reason',
'capabilities': 'fake capabilities',
'disable_rollback': 'fake value',
'timeout_mins': 'fake timeout',
'stack_owner': 'fake owner',
'parent': 'fake parent',
'stack_user_project_id': 'fake project id',
'tags': 'fake tags',
'barlog': 'you shall not pass!'
}
req = self._get('/stacks', params=params)
mock_call.return_value = []
@ -333,15 +353,18 @@ class StackControllerTest(tools.ControllerTest, common.HeatTestCase):
self.assertIn('filters', engine_args)
filters = engine_args['filters']
self.assertEqual(7, len(filters))
self.assertIn('id', filters)
self.assertIn('status', filters)
self.assertIn('name', filters)
self.assertIn('action', filters)
self.assertIn('username', filters)
self.assertIn('tenant', filters)
self.assertIn('owner_id', filters)
self.assertNotIn('balrog', filters)
self.assertEqual(16, len(filters))
for key in ('id', 'status', 'name', 'action', 'username', 'tenant',
'owner_id', 'stack_name', 'stack_action', 'stack_status',
'stack_status_reason', 'disable_rollback', 'timeout_mins',
'stack_owner', 'parent', 'stack_user_project_id'):
self.assertIn(key, filters)
for key in ('stack_identity', 'creation_time', 'updated_time',
'deletion_time', 'notification_topics', 'description',
'template_description', 'parameters', 'outputs',
'capabilities', 'tags', 'barlog'):
self.assertNotIn(key, filters)
def test_index_returns_stack_count_if_with_count_is_true(
self, mock_enforce):

123
heat/tests/test_engine_api_utils.py
View File

@ -1110,3 +1110,126 @@ class TestExtractArgs(common.HeatTestCase):
exc = self.assertRaises(ValueError, api.extract_args, p)
self.assertIn('Invalid tag, "tag2," contains a comma',
six.text_type(exc))
class TranslateFilterTest(common.HeatTestCase):
scenarios = [
(
'single+single',
dict(inputs={'stack_status': 'COMPLETE', 'status': 'FAILED'},
expected={'status': ['COMPLETE', 'FAILED']})
), (
'none+single',
dict(inputs={'name': 'n1'},
expected={'name': 'n1'})
), (
'single+none',
dict(inputs={'stack_name': 'n1'},
expected={'name': 'n1'})
), (
'none+list',
dict(inputs={'action': ['a1', 'a2']},
expected={'action': ['a1', 'a2']})
), (
'list+none',
dict(inputs={'stack_action': ['a1', 'a2']},
expected={'action': ['a1', 'a2']})
), (
'single+list',
dict(inputs={'stack_owner': 'u1', 'username': ['u2', 'u3']},
expected={'username': ['u1', 'u2', 'u3']})
), (
'list+single',
dict(inputs={'parent': ['s1', 's2'], 'owner_id': 's3'},
expected={'owner_id': ['s1', 's2', 's3']})
), (
'list+list',
dict(inputs={'stack_name': ['n1', 'n2'], 'name': ['n3', 'n4']},
expected={'name': ['n1', 'n2', 'n3', 'n4']})
), (
'full_status_split',
dict(inputs={'stack_status': 'CREATE_COMPLETE'},
expected={'action': 'CREATE', 'status': 'COMPLETE'})
), (
'full_status_split_merge',
dict(inputs={'stack_status': 'CREATE_COMPLETE',
'status': 'CREATE_FAILED'},
expected={'action': 'CREATE',
'status': ['COMPLETE', 'FAILED']})
), (
'action_status_merge',
dict(inputs={'action': ['UPDATE', 'CREATE'],
'status': 'CREATE_FAILED'},
expected={'action': ['CREATE', 'UPDATE'],
'status': 'FAILED'})
)
]
def test_stack_filter_translate(self):
actual = api.translate_filters(self.inputs)
self.assertEqual(self.expected, actual)
class ParseStatusTest(common.HeatTestCase):
scenarios = [
(
'single_bogus',
dict(inputs='bogus status',
expected=(set(), set()))
), (
'list_bogus',
dict(inputs=['foo', 'bar'],
expected=(set(), set()))
), (
'single_partial',
dict(inputs='COMPLETE',
expected=(set(), set(['COMPLETE'])))
), (
'multi_partial',
dict(inputs=['FAILED', 'COMPLETE'],
expected=(set(), set(['FAILED', 'COMPLETE'])))
), (
'multi_partial_dup',
dict(inputs=['FAILED', 'FAILED'],
expected=(set(), set(['FAILED'])))
), (
'single_full',
dict(inputs=['DELETE_FAILED'],
expected=(set(['DELETE']), set(['FAILED'])))
), (
'multi_full',
dict(inputs=['DELETE_FAILED', 'CREATE_COMPLETE'],
expected=(set(['CREATE', 'DELETE']),
set(['COMPLETE', 'FAILED'])))
), (
'mix_bogus_partial',
dict(inputs=['delete_failed', 'COMPLETE'],
expected=(set(), set(['COMPLETE'])))
), (
'mix_bogus_full',
dict(inputs=['delete_failed', 'action_COMPLETE'],
expected=(set(['action']), set(['COMPLETE'])))
), (
'mix_bogus_full_incomplete',
dict(inputs=['delete_failed', '_COMPLETE'],
expected=(set(), set(['COMPLETE'])))
), (
'mix_partial_full',
dict(inputs=['FAILED', 'b_COMPLETE'],
expected=(set(['b']),
set(['COMPLETE', 'FAILED'])))
), (
'mix_full_dup',
dict(inputs=['a_FAILED', 'a_COMPLETE'],
expected=(set(['a']),
set(['COMPLETE', 'FAILED'])))
), (
'mix_full_dup_2',
dict(inputs=['a_FAILED', 'b_FAILED'],
expected=(set(['a', 'b']), set(['FAILED'])))
)
]
def test_stack_parse_status(self):
actual = api._parse_object_status(self.inputs)
self.assertEqual(self.expected, actual)

21
heat/tests/test_engine_service.py
View File

@ -1709,6 +1709,27 @@ class StackServiceTest(common.HeatTestCase):
mock.ANY,
)
@mock.patch.object(stack_object.Stack, 'get_all')
def test_stack_list_passes_filter_translated(self, mock_stack_get_all):
filters = {'stack_name': 'bar'}
self.eng.list_stacks(self.ctx, filters=filters)
translated = {'name': 'bar'}
mock_stack_get_all.assert_called_once_with(mock.ANY,
mock.ANY,
mock.ANY,
mock.ANY,
mock.ANY,
translated,
mock.ANY,
mock.ANY,
mock.ANY,
mock.ANY,
mock.ANY,
mock.ANY,
mock.ANY,
mock.ANY,
)
@mock.patch.object(stack_object.Stack, 'get_all')
def test_stack_list_tenant_safe_defaults_to_true(self, mock_stack_get_all):
self.eng.list_stacks(self.ctx)